Reapplied changes from pybind#3674

laramiel · laramiel · commit fe02513f90d9 · 2022-04-11T12:56:38.000-07:00
diff --git a/docs/advanced/cast/custom.rst b/docs/advanced/cast/custom.rst
@@ -31,63 +31,81 @@ The following Python snippet demonstrates the intended usage from the Python sid
 
     print(A())
 
-To register the necessary conversion routines, it is necessary to add an
-instantiation of the ``pybind11::detail::type_caster<T>`` template.
-Although this is an implementation detail, adding an instantiation of this
-type is explicitly allowed.
+To register the necessary conversion routines, it is necessary to define a
+caster class, and register it with the other pybind11 casters:
 
 .. code-block:: cpp
 
-    namespace pybind11 { namespace detail {
-        template <> struct type_caster<inty> {
-        public:
-            /**
-             * This macro establishes the name 'inty' in
-             * function signatures and declares a local variable
-             * 'value' of type inty
-             */
-            PYBIND11_TYPE_CASTER(inty, const_name("inty"));
-
-            /**
-             * Conversion part 1 (Python->C++): convert a PyObject into a inty
-             * instance or return false upon failure. The second argument
-             * indicates whether implicit conversions should be applied.
-             */
-            bool load(handle src, bool) {
-                /* Extract PyObject from handle */
-                PyObject *source = src.ptr();
-                /* Try converting into a Python integer value */
-                PyObject *tmp = PyNumber_Long(source);
-                if (!tmp)
-                    return false;
-                /* Now try to convert into a C++ int */
-                value.long_value = PyLong_AsLong(tmp);
-                Py_DECREF(tmp);
-                /* Ensure return code was OK (to avoid out-of-range errors etc) */
-                return !(value.long_value == -1 && !PyErr_Occurred());
-            }
-
-            /**
-             * Conversion part 2 (C++ -> Python): convert an inty instance into
-             * a Python object. The second and third arguments are used to
-             * indicate the return value policy and parent object (for
-             * ``return_value_policy::reference_internal``) and are generally
-             * ignored by implicit casters.
-             */
-            static handle cast(inty src, return_value_policy /* policy */, handle /* parent */) {
-                return PyLong_FromLong(src.long_value);
-            }
-        };
-    }} // namespace pybind11::detail
+    struct inty_type_caster {
+    public:
+        /**
+         * This macro establishes the name 'inty' in
+         * function signatures and declares a local variable
+         * 'value' of type inty
+         */
+        PYBIND11_TYPE_CASTER(inty, const_name("inty"));
+        /**
+         * Conversion part 1 (Python->C++): convert a PyObject into a inty
+         * instance or return false upon failure. The second argument
+         * indicates whether implicit conversions should be applied.
+         */
+        bool load(handle src, bool) {
+            /* Extract PyObject from handle */
+            PyObject *source = src.ptr();
+            /* Try converting into a Python integer value */
+            PyObject *tmp = PyNumber_Long(source);
+            if (!tmp)
+                return false;
+            /* Now try to convert into a C++ int */
+            value.long_value = PyLong_AsLong(tmp);
+            Py_DECREF(tmp);
+            /* Ensure return code was OK (to avoid out-of-range errors etc) */
+            return !(value.long_value == -1 && !PyErr_Occurred());
+        }
+        /**
+         * Conversion part 2 (C++ -> Python): convert an inty instance into
+         * a Python object. The second and third arguments are used to
+         * indicate the return value policy and parent object (for
+         * ``return_value_policy::reference_internal``) and are generally
+         * ignored by implicit casters.
+         */
+        static handle cast(inty src, return_value_policy /* policy */, handle /* parent */) {
+            return PyLong_FromLong(src.long_value);
+        }
+    };
 
 .. note::
 
-    A ``type_caster<T>`` defined with ``PYBIND11_TYPE_CASTER(T, ...)`` requires
+    A caster class defined with ``PYBIND11_TYPE_CASTER(T, ...)`` requires
     that ``T`` is default-constructible (``value`` is first default constructed
     and then ``load()`` assigns to it).
 
-.. warning::
+The caster defined above must be registered with pybind11.
+There are two ways to do it:
 
-    When using custom type casters, it's important to declare them consistently
-    in every compilation unit of the Python extension module. Otherwise,
-    undefined behavior can ensue.
+* As an instantiation of the ``pybind11::detail::type_caster<T>`` template.
+  Although this is an implementation detail, adding an instantiation of this
+  type is explicitly allowed:
+
+  .. code-block:: cpp
+
+      namespace pybind11 { namespace detail {
+          template <> struct type_caster<inty> : inty_type_caster {};
+      }} // namespace pybind11::detail
+
+  .. warning::
+
+      When using this method, it's important to declare them consistently
+      in every compilation unit of the Python extension module. Otherwise,
+      undefined behavior can ensue.
+
+* When you own the namespace where the type is defined, the preferred method
+  is to *declare* a function named ``pybind11_select_caster``, its only purpose
+  is to associate the C++ type with its caster class:
+
+  .. code-block:: cpp
+
+      inty_type_caster pybind11_select_caster(inty*);
+
+  The argument is a *pointer* to the C++ type, the return type is the
+  caster type. This function has no implementation!
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
@@ -33,8 +33,34 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 
 template <typename type, typename SFINAE = void>
 class type_caster : public type_caster_base<type> {};
+
+template <typename IntrinsicType> type_caster<IntrinsicType> pybind11_select_caster(IntrinsicType *);
+
+// MSVC 2015 generates an internal compiler error for the common code (in the #else branch below).
+// MSVC 2017 in C++14 mode produces incorrect code, leading to a tests/test_stl runtime failure.
+// Luckily, the workaround for MSVC 2015 also resolves the MSVC 2017 C++14 runtime failure.
+#if defined(_MSC_VER) && (_MSC_VER < 1910 || (_MSC_VER < 1920 && !defined(PYBIND11_CPP17)))
+
+template <typename IntrinsicType, typename SFINAE = void>
+struct make_caster_impl;
+
+template <typename IntrinsicType>
+struct make_caster_impl<IntrinsicType, enable_if_t< std::is_arithmetic<IntrinsicType>::value>>
+: type_caster<IntrinsicType> {};
+
+template <typename IntrinsicType>
+struct make_caster_impl<IntrinsicType, enable_if_t<!std::is_arithmetic<IntrinsicType>::value>>
+: decltype(pybind11_select_caster(static_cast<IntrinsicType *>(nullptr))) {};
+
 template <typename type>
-using make_caster = type_caster<intrinsic_t<type>>;
+using make_caster = make_caster_impl<intrinsic_t<type>>;
+
+#else
+
+template <typename type>
+using make_caster = decltype(pybind11_select_caster(static_cast<intrinsic_t<type> *>(nullptr)));
+
+#endif
 
 // Shortcut for calling a caster's `cast_op_type` cast operator for casting a type_caster to a T
 template <typename T>
@@ -998,8 +1024,8 @@ struct return_value_policy_override<
 };
 
 // Basic python -> C++ casting; throws if casting fails
-template <typename T, typename SFINAE>
-type_caster<T, SFINAE> &load_type(type_caster<T, SFINAE> &conv, const handle &handle) {
+template <typename T, typename Caster>
+Caster &load_type(Caster& conv, const handle& handle) {
     if (!conv.load(handle, true)) {
 #if defined(NDEBUG)
         throw cast_error(
@@ -1012,11 +1038,17 @@ type_caster<T, SFINAE> &load_type(type_caster<T, SFINAE> &conv, const handle &ha
     }
     return conv;
 }
+
+template <typename T, typename SFINAE>
+type_caster<T, SFINAE>& load_type(type_caster<T, SFINAE>& conv, const handle& handle) {
+    return load_type<T, type_caster<T, SFINAE>>(conv, handle);
+}
+
 // Wrapper around the above that also constructs and returns a type_caster
 template <typename T>
 make_caster<T> load_type(const handle &handle) {
     make_caster<T> conv;
-    load_type(conv, handle);
+    load_type<T>(conv, handle);
     return conv;
 }
 
@@ -1143,7 +1175,7 @@ using override_caster_t = conditional_t<cast_is_temporary_value_reference<ret_ty
 template <typename T>
 enable_if_t<cast_is_temporary_value_reference<T>::value, T> cast_ref(object &&o,
                                                                      make_caster<T> &caster) {
-    return cast_op<T>(load_type(caster, o));
+    return cast_op<T>(load_type<T>(caster, o));
 }
 template <typename T>
 enable_if_t<!cast_is_temporary_value_reference<T>::value, T> cast_ref(object &&,
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -137,6 +137,8 @@ set(PYBIND11_TEST_FILES
     test_iostream
     test_kwargs_and_defaults
     test_local_bindings
+    test_make_caster_adl
+    test_make_caster_adl_alt
     test_methods_and_attributes
     test_modules
     test_multiple_inheritance
diff --git a/tests/test_make_caster_adl.cpp b/tests/test_make_caster_adl.cpp
@@ -0,0 +1,89 @@
+// adl = Argument Dependent Lookup
+
+#include "pybind11_tests.h"
+
+namespace have_a_ns {
+struct type_mock {};
+struct mock_caster {
+    static int num() { return 101; }
+};
+mock_caster pybind11_select_caster(type_mock *);
+} // namespace have_a_ns
+
+// namespace global {
+struct global_ns_type_mock {};
+struct global_ns_mock_caster {
+    static int num() { return 202; }
+};
+global_ns_mock_caster pybind11_select_caster(global_ns_type_mock *);
+// } // namespace global
+
+namespace {
+struct unnamed_ns_type_mock {};
+struct unnamed_ns_mock_caster {
+    static int num() { return 303; }
+};
+unnamed_ns_mock_caster pybind11_select_caster(unnamed_ns_type_mock *);
+#if !defined(_MSC_VER)
+// Dummy implementation, purely to avoid compiler warnings (unused function).
+// Easier than managing compiler-specific pragmas for warning suppression.
+// MSVC happens to not generate any warnings. Leveraging that to prove that
+// this test actually works without an implementation.
+unnamed_ns_mock_caster pybind11_select_caster(unnamed_ns_type_mock *) {
+    return unnamed_ns_mock_caster{};
+}
+#endif
+} // namespace
+
+namespace mrc_ns { // minimal real caster
+
+struct type_mrc {
+    int value = -9999;
+};
+
+struct minimal_real_caster {
+    static constexpr auto name = py::detail::const_name<type_mrc>();
+
+    static py::handle
+    cast(type_mrc const &src, py::return_value_policy /*policy*/, py::handle /*parent*/) {
+        return py::int_(src.value + 1000).release();
+    }
+
+    // Maximizing simplicity. This will go terribly wrong for other arg types.
+    template <typename>
+    using cast_op_type = const type_mrc &;
+
+    // NOLINTNEXTLINE(google-explicit-constructor)
+    operator type_mrc const &() {
+        static type_mrc obj;
+        obj.value = 404;
+        return obj;
+    }
+
+    bool load(py::handle src, bool /*convert*/) {
+        // Only accepts str, but the value is ignored.
+        return py::isinstance<py::str>(src);
+    }
+};
+
+minimal_real_caster pybind11_select_caster(type_mrc *);
+
+} // namespace mrc_ns
+
+TEST_SUBMODULE(make_caster_adl, m) {
+    m.def("have_a_ns_num", []() { return py::detail::make_caster<have_a_ns::type_mock>::num(); });
+    m.def("global_ns_num", []() { return py::detail::make_caster<global_ns_type_mock>::num(); });
+    m.def("unnamed_ns_num", []() { return py::detail::make_caster<unnamed_ns_type_mock>::num(); });
+
+    m.def("mrc_return", []() {
+        mrc_ns::type_mrc obj;
+        obj.value = 505;
+        return obj;
+    });
+    m.def("mrc_arg", [](mrc_ns::type_mrc const &obj) { return obj.value + 2000; });
+
+#if !defined(_MSC_VER)
+    // Dummy call, purely to avoid compiler warnings (unused function).
+    (void) pybind11_select_caster(static_cast<unnamed_ns_type_mock *>(nullptr));
+#endif
+}
diff --git a/tests/test_make_caster_adl.py b/tests/test_make_caster_adl.py
@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+import pytest
+
+from pybind11_tests import make_caster_adl as m
+from pybind11_tests import make_caster_adl_alt as m_alt
+
+
+def test_mock_casters():
+    assert m.have_a_ns_num() == 101
+    assert m.global_ns_num() == 202
+    assert m.unnamed_ns_num() == 303
+
+
+def test_mock_casters_alt():
+    assert m_alt.have_a_ns_num() == 121
+
+
+def test_minimal_real_caster():
+    assert m.mrc_return() == 1505
+    assert m.mrc_arg(u"ignored") == 2404
+    with pytest.raises(TypeError) as excinfo:
+        m.mrc_arg(None)
+    assert "(arg0: mrc_ns::type_mrc) -> int" in str(excinfo.value)
diff --git a/tests/test_make_caster_adl_alt.cpp b/tests/test_make_caster_adl_alt.cpp
@@ -0,0 +1,13 @@
+#include "pybind11_tests.h"
+
+namespace have_a_ns {
+struct type_mock {};
+struct mock_caster_alt {
+    static int num() { return 121; }
+};
+mock_caster_alt pybind11_select_caster(type_mock *);
+} // namespace have_a_ns
+
+TEST_SUBMODULE(make_caster_adl_alt, m) {
+    m.def("have_a_ns_num", []() { return py::detail::make_caster<have_a_ns::type_mock>::num(); });
+}
diff --git a/tests/test_make_caster_adl_alt.py b/tests/test_make_caster_adl_alt.py
@@ -0,0 +1,7 @@
+# -*- coding: utf-8 -*-
+
+from pybind11_tests import make_caster_adl_alt as m
+
+
+def test_mock_casters():
+    assert m.have_a_ns_num() == 121
