Skip to content
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

maint: disable implicit conversion of 0 to handle #4006

Conversation

Skylion007
Copy link
Collaborator

Description

  • Disable implicit conversion of int to pyobject* to handle.

Suggested changelog entry:

@@ -214,6 +214,7 @@ class handle : public detail::object_api<handle> {
/// Creates a ``handle`` from the given raw Python object pointer
// NOLINTNEXTLINE(google-explicit-constructor)
handle(PyObject *ptr) : m_ptr(ptr) {} // Allow implicit conversion from PyObject*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is a job for enable_if?
I'd bet though enable_if PyObject* won't work, is_pointer or similar might. Or maybe to be fancy, look for ob_refcnt in the pointee?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwgk Hmm... can't figure out the way to construct the enable_if_t.

Copy link
Collaborator Author

@Skylion007 Skylion007 Jun 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This strategy doesn't work because it causes ambiguous overloads, which is silly since the func is deleted.

@rwgk
Copy link
Collaborator

rwgk commented Jun 14, 2022

It's a can of worms indeed, but I got pretty far:

rwgk.c.googlers.com:~/forked/pybind11 $ git diff
diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index f9625e77..370b3f9a 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -213,7 +213,13 @@ public:
     handle() = default;
     /// Creates a ``handle`` from the given raw Python object pointer
     // NOLINTNEXTLINE(google-explicit-constructor)
-    handle(PyObject *ptr) : m_ptr(ptr) {} // Allow implicit conversion from PyObject*
+    template <typename T, detail::enable_if_t<std::is_pointer<T>::value || std::is_same<T, std::nullptr_t>::value, int> = 0>
+    handle(T ptr) : m_ptr(ptr) {} // Allow implicit conversion from PyObject*
+
+    handle(const handle &) = default;
+    handle(handle &&) = default;
+    handle& operator=(const handle &) = default;
+    handle& operator=(handle &&) = default;

     /// Return the underlying ``PyObject *`` pointer
     PyObject *ptr() const { return m_ptr; }
diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp
index 69ddbe1e..f8a777e4 100644
--- a/tests/test_numpy_array.cpp
+++ b/tests/test_numpy_array.cpp
@@ -218,7 +218,7 @@ TEST_SUBMODULE(numpy_array, sm) {
     // test_empty_shaped_array
     sm.def("make_empty_shaped_array", [] { return py::array(py::dtype("f"), {}, {}); });
     // test numpy scalars (empty shape, ndim==0)
-    sm.def("scalar_int", []() { return py::array(py::dtype("i"), {}, {}, &data_i); });
+    sm.def("scalar_int", []() { return py::array(py::dtype("i"), {}, {}/*, &data_i*/); });

     // test_wrap
     sm.def("wrap", [](const py::array &a) {

There is only one subtest that does not pass (test_make_empty_shaped_array) with this diff. Is there a chance you could help debugging that?

@Skylion007
Copy link
Collaborator Author

Closing in favor of #4008

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants