-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add py::nullptr_default_arg
(intended for PyObject *
arguments)
#30081
Changes from 5 commits
d25ede1
06c0f99
558733a
dbe249f
8e681ff
6546148
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,4 +127,28 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { | |
(void) py::cast(*ptr); | ||
} | ||
#endif | ||
|
||
// This test exercises functionality (`py::cast<PyObject *>(handle_nullptr)`) | ||
// that is needed indirectly below in py_arg_handle_nullptr. | ||
m.def("pyobject_ptr_from_handle_nullptr", []() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not very clear at first glance how this handle_nullptr logic is used with the rest of the change. Could you elaborate a bit more on this? Also surprising to me is that if a py::handle can already hold a nullptr, then why do we need to add another value_is_nullptr member to the arg struct, which already hold a py::handle member? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yes, the information you're looking for was only in a commit message before (pybind11 master PR used to start this work, but closed already): I added a comment to explain. The core idea is to cleanly exercise a critical sub-feature in isolation. It did not need production code changes, but I don't think it is explicitly exercised anywhere else already. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! |
||
py::handle handle_nullptr; | ||
if (handle_nullptr.ptr() != nullptr) { | ||
return "UNEXPECTED: handle_nullptr.ptr() != nullptr"; | ||
} | ||
auto *pyobject_ptr_from_handle = py::cast<PyObject *>(handle_nullptr); | ||
if (pyobject_ptr_from_handle != nullptr) { | ||
return "UNEXPECTED: pyobject_ptr_from_handle != nullptr"; | ||
} | ||
return "SUCCESS"; | ||
}); | ||
|
||
m.def( | ||
"py_arg_handle_nullptr", | ||
[](PyObject *ptr) { | ||
if (ptr == nullptr) { | ||
return "ptr == nullptr"; | ||
} | ||
return py::detail::obj_class_name(ptr); | ||
}, | ||
py::arg("ptr") = py::nullptr_default_arg()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which of the case does True correspond to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, thanks. I made the comment more precise.