From a126d9b992b15a3e1c3654b6480d0bb06676bd12 Mon Sep 17 00:00:00 2001 From: Amaury Forgeot d'Arc Date: Tue, 25 Jan 2022 17:08:52 +0100 Subject: [PATCH 01/25] Function pybind11_select_caster uses ADL to select the caster. - There is no ODR violation when different translation units define different conversions. - The declarations need to be defined in the original namespace. --- include/pybind11/cast.h | 44 ++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 165102443c..3c267dd61b 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -31,7 +31,10 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) template class type_caster : public type_caster_base { }; -template using make_caster = type_caster>; + +template type_caster> pybind11_select_caster(type); + +template using make_caster = decltype(pybind11_select_caster(static_cast *>(nullptr))); // Shortcut for calling a caster's `cast_op_type` cast operator for casting a type_caster to a T template typename make_caster::template cast_op_type cast_op(make_caster &caster) { @@ -443,21 +446,41 @@ template struct string_caster { bool load_bytes(enable_if_t::value, handle>) { return false; } }; +PYBIND11_NAMESPACE_END(detail) +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) + +namespace std { + +// pybind11::detail::string_caster +// pybind11_select_caster(std::string*); template -struct type_caster, enable_if_t::value>> - : string_caster> {}; +pybind11::detail::enable_if_t< + pybind11::detail::is_std_char_type::value, + pybind11::detail::string_caster< + std::basic_string>> +pybind11_select_caster(std::basic_string*); #ifdef PYBIND11_HAS_STRING_VIEW +// pybind11::detail::string_caster +// pybind11_select_caster(std::string_view*); template -struct type_caster, enable_if_t::value>> - : string_caster, true> {}; +pybind11::detail::enable_if_t< + pybind11::detail::is_std_char_type::value, + pybind11::detail::string_caster< + std::basic_string_view, true>> +pybind11_select_caster(std::basic_string_view*); #endif +} // namespace std + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +PYBIND11_NAMESPACE_BEGIN(detail) + // Type caster for C-style strings. We basically use a std::string type caster, but also add the // ability to use None as a nullptr char* (which the string caster doesn't allow). template struct type_caster::value>> { using StringType = std::basic_string; - using StringCaster = type_caster; + using StringCaster = make_caster; StringCaster str_caster; bool none = false; CharT one_char = 0; @@ -863,7 +886,7 @@ template struct return_value_policy_override C++ casting; throws if casting fails -template type_caster &load_type(type_caster &conv, const handle &handle) { +template caster& load_type(caster& conv, const handle& handle) { if (!conv.load(handle, true)) { #if defined(NDEBUG) throw cast_error("Unable to cast Python instance to C++ type (compile in debug mode for details)"); @@ -874,10 +897,13 @@ template type_caster &load_type(type_ca } return conv; } +template type_caster &load_type(type_caster &conv, const handle &handle) { + return load_type>(conv, handle); +} // Wrapper around the above that also constructs and returns a type_caster template make_caster load_type(const handle &handle) { make_caster conv; - load_type(conv, handle); + load_type(conv, handle); return conv; } @@ -964,7 +990,7 @@ template using override_caster_t = conditional_t< // Trampoline use: for reference/pointer types to value-converted values, we do a value cast, then // store the result in the given variable. For other types, this is a no-op. template enable_if_t::value, T> cast_ref(object &&o, make_caster &caster) { - return cast_op(load_type(caster, o)); + return cast_op(load_type(caster, o)); } template enable_if_t::value, T> cast_ref(object &&, override_unused &) { pybind11_fail("Internal error: cast_ref fallback invoked"); } From 5cfd67b5e7ec25a9e63ca17c46f152fdd034a9c1 Mon Sep 17 00:00:00 2001 From: Amaury Forgeot d'Arc Date: Tue, 25 Jan 2022 19:17:23 +0100 Subject: [PATCH 02/25] Add documentation.: --- docs/advanced/cast/custom.rst | 119 ++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 50 deletions(-) diff --git a/docs/advanced/cast/custom.rst b/docs/advanced/cast/custom.rst index 1df4d3e14b..7e4bcabb6b 100644 --- a/docs/advanced/cast/custom.rst +++ b/docs/advanced/cast/custom.rst @@ -31,63 +31,82 @@ 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`` 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 +`type_caster` class, and register it with the other pybind11 casters: .. code-block:: cpp - namespace pybind11 { namespace detail { - template <> struct 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); - } - }; - }} // 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`` defined with ``PYBIND11_TYPE_CASTER(T, ...)`` requires + A `type_caster` 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 `type_caster` defined above must be registered with pybind11. +There are two ways to do this: - 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`` 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_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. + +* The preferred method is to *declare* a function named `pybind11_declare_caster`, + its only purpose is to associate the C++ type with its caster class: + + .. code-block:: cpp + + inty_type_caster pybind11_declare_caster(inty*); + + The argument is a *pointer* to the C++ type, the return type is the + `caster_type`. This function has no implementation! From dfd4bf0ed91fef0bd670ba55d05bf4a67fb46976 Mon Sep 17 00:00:00 2001 From: Amaury Forgeot d'Arc Date: Tue, 25 Jan 2022 19:19:52 +0100 Subject: [PATCH 03/25] Use correct function name in docs. --- docs/advanced/cast/custom.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/advanced/cast/custom.rst b/docs/advanced/cast/custom.rst index 7e4bcabb6b..028b10baa9 100644 --- a/docs/advanced/cast/custom.rst +++ b/docs/advanced/cast/custom.rst @@ -101,12 +101,12 @@ There are two ways to do this: in every compilation unit of the Python extension module. Otherwise, undefined behavior can ensue. -* The preferred method is to *declare* a function named `pybind11_declare_caster`, +* 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_declare_caster(inty*); + 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! From 43a065adaa6157827b190484558e8f18ec6ee819 Mon Sep 17 00:00:00 2001 From: Amaury Forgeot d'Arc Date: Tue, 25 Jan 2022 23:39:34 +0100 Subject: [PATCH 04/25] Attempt to fix compilation error on MSVC 2015 "explicit specialization 'void pybind11::detail::cast_safe(pybind11::object &&)' is not a specialization of a function template" --- include/pybind11/cast.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 3c267dd61b..d65cca1ec7 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -998,11 +998,16 @@ template enable_if_t::value, // Trampoline use: Having a pybind11::cast with an invalid reference type is going to static_assert, even // though if it's in dead code, so we provide a "trampoline" to pybind11::cast that only does anything in // cases where pybind11::cast is valid. -template enable_if_t::value, T> cast_safe(object &&o) { - return pybind11::cast(std::move(o)); } -template enable_if_t::value, T> cast_safe(object &&) { - pybind11_fail("Internal error: cast_safe fallback invoked"); } -template <> inline void cast_safe(object &&) {} +template +enable_if_t> && + !cast_is_temporary_value_reference::value, T> +cast_safe(object &&o) { return pybind11::cast(std::move(o)); } +template +enable_if_t::value, T> +cast_safe(object &&) { pybind11_fail("Internal error: cast_safe fallback invoked"); } +template +enable_if_t>, void> +cast_safe(object &&) {} PYBIND11_NAMESPACE_END(detail) From 28cef75bb53b0d6e09622cf2f5f359a6cb4fd83a Mon Sep 17 00:00:00 2001 From: Amaury Forgeot d'Arc Date: Tue, 25 Jan 2022 23:46:10 +0100 Subject: [PATCH 05/25] Address rst conformance checks. --- docs/advanced/cast/custom.rst | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/advanced/cast/custom.rst b/docs/advanced/cast/custom.rst index 028b10baa9..9dbd0f4482 100644 --- a/docs/advanced/cast/custom.rst +++ b/docs/advanced/cast/custom.rst @@ -32,7 +32,7 @@ The following Python snippet demonstrates the intended usage from the Python sid print(A()) To register the necessary conversion routines, it is necessary to define a -`type_caster` class, and register it with the other pybind11 casters: +caster class, and register it with the other pybind11 casters: .. code-block:: cpp @@ -78,12 +78,12 @@ To register the necessary conversion routines, it is necessary to define a .. note:: - A `type_caster` 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). -The `type_caster` defined above must be registered with pybind11. -There are two ways to do this: +The caster defined above must be registered with pybind11. +There are two ways to do it: * As an instantiation of the ``pybind11::detail::type_caster`` template. Although this is an implementation detail, adding an instantiation of this @@ -101,12 +101,13 @@ There are two ways to do this: in every compilation unit of the Python extension module. Otherwise, undefined behavior can ensue. -* 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: +* 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! + caster type. This function has no implementation! From 9100a05dddb63bff69c51e3cbbca00efab2c38c0 Mon Sep 17 00:00:00 2001 From: Amaury Forgeot d'Arc Date: Wed, 26 Jan 2022 00:28:08 +0100 Subject: [PATCH 06/25] Fix compilation. --- include/pybind11/cast.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index d65cca1ec7..89a6542923 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -999,15 +999,15 @@ template enable_if_t::value, // though if it's in dead code, so we provide a "trampoline" to pybind11::cast that only does anything in // cases where pybind11::cast is valid. template -enable_if_t> && +enable_if_t>::value && !cast_is_temporary_value_reference::value, T> cast_safe(object &&o) { return pybind11::cast(std::move(o)); } template enable_if_t::value, T> cast_safe(object &&) { pybind11_fail("Internal error: cast_safe fallback invoked"); } template -enable_if_t>, void> -cast_safe(object &&) {} +enable_if_t>::value, void> +cast_safe(object &&) {} PYBIND11_NAMESPACE_END(detail) From 13a63adde92adf3cd93796f04f56a5968ef48078 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 31 Jan 2022 18:24:14 -0800 Subject: [PATCH 07/25] Undoing std::string, std::string_view caster changes. --- include/pybind11/cast.h | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 89a6542923..b900c14818 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -446,36 +446,16 @@ template struct string_caster { bool load_bytes(enable_if_t::value, handle>) { return false; } }; -PYBIND11_NAMESPACE_END(detail) -PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) - -namespace std { - -// pybind11::detail::string_caster -// pybind11_select_caster(std::string*); template -pybind11::detail::enable_if_t< - pybind11::detail::is_std_char_type::value, - pybind11::detail::string_caster< - std::basic_string>> -pybind11_select_caster(std::basic_string*); +struct type_caster, enable_if_t::value>> + : string_caster> {}; #ifdef PYBIND11_HAS_STRING_VIEW -// pybind11::detail::string_caster -// pybind11_select_caster(std::string_view*); template -pybind11::detail::enable_if_t< - pybind11::detail::is_std_char_type::value, - pybind11::detail::string_caster< - std::basic_string_view, true>> -pybind11_select_caster(std::basic_string_view*); +struct type_caster, enable_if_t::value>> + : string_caster, true> {}; #endif -} // namespace std - -PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) -PYBIND11_NAMESPACE_BEGIN(detail) - // Type caster for C-style strings. We basically use a std::string type caster, but also add the // ability to use None as a nullptr char* (which the string caster doesn't allow). template struct type_caster::value>> { From 222d86700a0baab49ffd3184ccda26b5041e60bf Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 31 Jan 2022 19:01:15 -0800 Subject: [PATCH 08/25] Adding ReferenceSensitiveOptional copy/move/operator= as suggested by @laramiel --- tests/test_stl.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index bc5c6553a2..cc0bd827be 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -125,6 +125,12 @@ class ReferenceSensitiveOptional { ReferenceSensitiveOptional(const T& value) : storage{value} {} // NOLINTNEXTLINE(google-explicit-constructor) ReferenceSensitiveOptional(T&& value) : storage{std::move(value)} {} + + ReferenceSensitiveOptional(const ReferenceSensitiveOptional&) = default; + ReferenceSensitiveOptional(ReferenceSensitiveOptional&&) = default; + ReferenceSensitiveOptional& operator=(const ReferenceSensitiveOptional&) = default; + ReferenceSensitiveOptional& operator=(ReferenceSensitiveOptional&&) = default; + ReferenceSensitiveOptional& operator=(const T& value) { storage = {value}; return *this; From 59fadb3798ac985487076e77a7b545b21035ba78 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 31 Jan 2022 20:12:18 -0800 Subject: [PATCH 09/25] noexcept to make clang-tidy happy; explicit py::return_value_policy::move --- tests/test_stl.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index cc0bd827be..58abd60d3a 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -127,9 +127,9 @@ class ReferenceSensitiveOptional { ReferenceSensitiveOptional(T&& value) : storage{std::move(value)} {} ReferenceSensitiveOptional(const ReferenceSensitiveOptional&) = default; - ReferenceSensitiveOptional(ReferenceSensitiveOptional&&) = default; + ReferenceSensitiveOptional(ReferenceSensitiveOptional&&) noexcept = default; ReferenceSensitiveOptional& operator=(const ReferenceSensitiveOptional&) = default; - ReferenceSensitiveOptional& operator=(ReferenceSensitiveOptional&&) = default; + ReferenceSensitiveOptional& operator=(ReferenceSensitiveOptional&&) noexcept = default; ReferenceSensitiveOptional& operator=(const T& value) { storage = {value}; @@ -424,7 +424,7 @@ TEST_SUBMODULE(stl, m) { pybind11::class_(m, "OptionalRefSensitiveProperties") .def(pybind11::init<>()) .def_property_readonly("access_by_ref", &opt_refsensitive_props::access_by_ref) - .def_property_readonly("access_by_copy", &opt_refsensitive_props::access_by_copy); + .def_property_readonly("access_by_copy", &opt_refsensitive_props::access_by_copy, py::return_value_policy::move); #ifdef PYBIND11_HAS_FILESYSTEM // test_fs_path From 74d9ee442fb715d88a0d3ea4acd8c71ee22790fe Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 31 Jan 2022 22:48:53 -0800 Subject: [PATCH 10/25] Disabling added copy/move/operator= --- tests/test_stl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 58abd60d3a..a794dbe8c6 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -125,12 +125,12 @@ class ReferenceSensitiveOptional { ReferenceSensitiveOptional(const T& value) : storage{value} {} // NOLINTNEXTLINE(google-explicit-constructor) ReferenceSensitiveOptional(T&& value) : storage{std::move(value)} {} - +#if 0 ReferenceSensitiveOptional(const ReferenceSensitiveOptional&) = default; ReferenceSensitiveOptional(ReferenceSensitiveOptional&&) noexcept = default; ReferenceSensitiveOptional& operator=(const ReferenceSensitiveOptional&) = default; ReferenceSensitiveOptional& operator=(ReferenceSensitiveOptional&&) noexcept = default; - +#endif ReferenceSensitiveOptional& operator=(const T& value) { storage = {value}; return *this; From 09c664a605052c8c164dbe2d1b56f289bd792012 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 31 Jan 2022 23:52:40 -0800 Subject: [PATCH 11/25] Trying pybind11_select_caster(IntrinsicType *); --- include/pybind11/cast.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index b900c14818..565f31fdd7 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -32,9 +32,9 @@ PYBIND11_NAMESPACE_BEGIN(detail) template class type_caster : public type_caster_base { }; -template type_caster> pybind11_select_caster(type); +template type_caster pybind11_select_caster(IntrinsicType *); -template using make_caster = decltype(pybind11_select_caster(static_cast *>(nullptr))); +template using make_caster = decltype(pybind11_select_caster(static_cast *>(nullptr))); // Shortcut for calling a caster's `cast_op_type` cast operator for casting a type_caster to a T template typename make_caster::template cast_op_type cast_op(make_caster &caster) { From f4e9248ca096b5348f4685d057df373d4c25d5fd Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 1 Feb 2022 00:37:12 -0800 Subject: [PATCH 12/25] make_caster_impl experiment --- include/pybind11/cast.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 565f31fdd7..bb3cbf3722 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -34,7 +34,19 @@ template class type_caster : public type template type_caster pybind11_select_caster(IntrinsicType *); -template using make_caster = decltype(pybind11_select_caster(static_cast *>(nullptr))); +template +struct make_caster_impl; + +template +struct make_caster_impl::value>::type> +: type_caster {}; + +template +struct make_caster_impl::value>::type> +: decltype(pybind11_select_caster(static_cast(nullptr))) {}; + +template +using make_caster = make_caster_impl>; // Shortcut for calling a caster's `cast_op_type` cast operator for casting a type_caster to a T template typename make_caster::template cast_op_type cast_op(make_caster &caster) { From 1c73b00caeb820a63ac55ca19f09973cf273d4cb Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 1 Feb 2022 00:56:51 -0800 Subject: [PATCH 13/25] test_make_caster_adl test_basic --- tests/CMakeLists.txt | 1 + tests/test_make_caster_adl.cpp | 22 ++++++++++++++++++++++ tests/test_make_caster_adl.py | 8 ++++++++ 3 files changed, 31 insertions(+) create mode 100644 tests/test_make_caster_adl.cpp create mode 100644 tests/test_make_caster_adl.py diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9040cf8c06..d2417de944 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -137,6 +137,7 @@ set(PYBIND11_TEST_FILES test_iostream test_kwargs_and_defaults test_local_bindings + test_make_caster_adl 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 new file mode 100644 index 0000000000..7757abc512 --- /dev/null +++ b/tests/test_make_caster_adl.cpp @@ -0,0 +1,22 @@ +#include "pybind11_tests.h" + +namespace adl_one { +struct type_one {}; +struct type_caster_one { + static int num() { return 101; } +}; +type_caster_one pybind11_select_caster(type_one *); +} // namespace adl_one + +namespace adl_two { +struct type_two {}; +struct type_caster_two { + static int num() { return 202; } +}; +type_caster_two pybind11_select_caster(type_two *); +} // namespace adl_two + +TEST_SUBMODULE(make_caster_adl, m) { + m.def("num_one", []() { return py::detail::make_caster::num(); }); + m.def("num_two", []() { return py::detail::make_caster::num(); }); +} diff --git a/tests/test_make_caster_adl.py b/tests/test_make_caster_adl.py new file mode 100644 index 0000000000..13a881479b --- /dev/null +++ b/tests/test_make_caster_adl.py @@ -0,0 +1,8 @@ +# -*- coding: utf-8 -*- + +from pybind11_tests import make_caster_adl as m + + +def test_basic(): + assert m.num_one() == 101 + assert m.num_two() == 202 From 62ca662eefa56d7531d7051fb0519aaae50e1c60 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 1 Feb 2022 01:28:11 -0800 Subject: [PATCH 14/25] Undoing all changes in tests/test_stl.cpp --- tests/test_stl.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index a794dbe8c6..bc5c6553a2 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -125,12 +125,6 @@ class ReferenceSensitiveOptional { ReferenceSensitiveOptional(const T& value) : storage{value} {} // NOLINTNEXTLINE(google-explicit-constructor) ReferenceSensitiveOptional(T&& value) : storage{std::move(value)} {} -#if 0 - ReferenceSensitiveOptional(const ReferenceSensitiveOptional&) = default; - ReferenceSensitiveOptional(ReferenceSensitiveOptional&&) noexcept = default; - ReferenceSensitiveOptional& operator=(const ReferenceSensitiveOptional&) = default; - ReferenceSensitiveOptional& operator=(ReferenceSensitiveOptional&&) noexcept = default; -#endif ReferenceSensitiveOptional& operator=(const T& value) { storage = {value}; return *this; @@ -424,7 +418,7 @@ TEST_SUBMODULE(stl, m) { pybind11::class_(m, "OptionalRefSensitiveProperties") .def(pybind11::init<>()) .def_property_readonly("access_by_ref", &opt_refsensitive_props::access_by_ref) - .def_property_readonly("access_by_copy", &opt_refsensitive_props::access_by_copy, py::return_value_policy::move); + .def_property_readonly("access_by_copy", &opt_refsensitive_props::access_by_copy); #ifdef PYBIND11_HAS_FILESYSTEM // test_fs_path From 7d2e5242f21f09221b1b9cfd185172d3cd581e9a Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 1 Feb 2022 17:43:18 -0800 Subject: [PATCH 15/25] Adding test_minimal_real_caster. Keeping only one mock_caster. --- tests/test_make_caster_adl.cpp | 60 ++++++++++++++++++++++++++-------- tests/test_make_caster_adl.py | 14 ++++++-- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/tests/test_make_caster_adl.cpp b/tests/test_make_caster_adl.cpp index 7757abc512..416cf62181 100644 --- a/tests/test_make_caster_adl.cpp +++ b/tests/test_make_caster_adl.cpp @@ -1,22 +1,56 @@ #include "pybind11_tests.h" -namespace adl_one { -struct type_one {}; -struct type_caster_one { +// adl = Argument Dependent Lookup + +namespace adl_mock { +struct type_mock {}; +struct mock_caster { static int num() { return 101; } }; -type_caster_one pybind11_select_caster(type_one *); -} // namespace adl_one +mock_caster pybind11_select_caster(type_mock *); +} // namespace adl_mock + +namespace adl_mrc { // minimal real caster -namespace adl_two { -struct type_two {}; -struct type_caster_two { - static int num() { return 202; } +struct type_mrc { + int value = -9999; }; -type_caster_two pybind11_select_caster(type_two *); -} // namespace adl_two + +struct minimal_real_caster { + static constexpr auto name = py::detail::const_name(); + + 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 + using cast_op_type = const type_mrc &; + + operator type_mrc const &() { + static type_mrc obj; + obj.value = 303; + return obj; + } + + bool load(py::handle src, bool /*convert*/) { + // Only accepts str, but the value is ignored. + return py::isinstance(src); + } +}; + +minimal_real_caster pybind11_select_caster(type_mrc *); + +} // namespace adl_mrc TEST_SUBMODULE(make_caster_adl, m) { - m.def("num_one", []() { return py::detail::make_caster::num(); }); - m.def("num_two", []() { return py::detail::make_caster::num(); }); + m.def("num_mock", []() { return py::detail::make_caster::num(); }); + + m.def("obj_mrc_return", []() { + adl_mrc::type_mrc obj; + obj.value = 404; + return obj; + }); + m.def("obj_mrc_arg", [](adl_mrc::type_mrc const &obj) { return obj.value + 2000; }); } diff --git a/tests/test_make_caster_adl.py b/tests/test_make_caster_adl.py index 13a881479b..e5d7b08d3c 100644 --- a/tests/test_make_caster_adl.py +++ b/tests/test_make_caster_adl.py @@ -1,8 +1,16 @@ # -*- coding: utf-8 -*- +import pytest from pybind11_tests import make_caster_adl as m -def test_basic(): - assert m.num_one() == 101 - assert m.num_two() == 202 +def test_mock_caster(): + assert m.num_mock() == 101 + + +def test_minimal_real_caster(): + assert m.obj_mrc_return() == 1404 + assert m.obj_mrc_arg("ignored") == 2303 + with pytest.raises(TypeError) as excinfo: + m.obj_mrc_arg(None) + assert "(arg0: adl_mrc::type_mrc) -> int" in str(excinfo.value) From 733d8b10d8109b38986c597c84ac6ad5b2addec0 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 1 Feb 2022 18:03:16 -0800 Subject: [PATCH 16/25] Python 2 compatibility. --- tests/test_make_caster_adl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_make_caster_adl.py b/tests/test_make_caster_adl.py index e5d7b08d3c..d43d00fe42 100644 --- a/tests/test_make_caster_adl.py +++ b/tests/test_make_caster_adl.py @@ -10,7 +10,7 @@ def test_mock_caster(): def test_minimal_real_caster(): assert m.obj_mrc_return() == 1404 - assert m.obj_mrc_arg("ignored") == 2303 + assert m.obj_mrc_arg(u"ignored") == 2303 with pytest.raises(TypeError) as excinfo: m.obj_mrc_arg(None) assert "(arg0: adl_mrc::type_mrc) -> int" in str(excinfo.value) From 6a2ce0c55b0cad7e5631f755c499fd9d2693fa9a Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 1 Feb 2022 18:04:43 -0800 Subject: [PATCH 17/25] Make clang-tidy happy. --- tests/test_make_caster_adl.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_make_caster_adl.cpp b/tests/test_make_caster_adl.cpp index 416cf62181..e854541390 100644 --- a/tests/test_make_caster_adl.cpp +++ b/tests/test_make_caster_adl.cpp @@ -28,6 +28,7 @@ struct minimal_real_caster { template using cast_op_type = const type_mrc &; + // NOLINTNEXTLINE(google-explicit-constructor) operator type_mrc const &() { static type_mrc obj; obj.value = 303; From 9caa19008b88f46b40fbe00861b8a419694f63d1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 1 Feb 2022 18:06:59 -0800 Subject: [PATCH 18/25] Adding dummy returned caster pointer to pybind11_select_caster() overloads. --- tests/test_make_caster_adl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_make_caster_adl.cpp b/tests/test_make_caster_adl.cpp index e854541390..c68a8d66ba 100644 --- a/tests/test_make_caster_adl.cpp +++ b/tests/test_make_caster_adl.cpp @@ -7,7 +7,7 @@ struct type_mock {}; struct mock_caster { static int num() { return 101; } }; -mock_caster pybind11_select_caster(type_mock *); +mock_caster pybind11_select_caster(type_mock *, mock_caster * = nullptr); } // namespace adl_mock namespace adl_mrc { // minimal real caster @@ -41,7 +41,7 @@ struct minimal_real_caster { } }; -minimal_real_caster pybind11_select_caster(type_mrc *); +minimal_real_caster pybind11_select_caster(type_mrc *, minimal_real_caster * = nullptr); } // namespace adl_mrc From f6f2ee212df208a00c789a24841e32c8bcff019a Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 2 Feb 2022 10:22:07 -0800 Subject: [PATCH 19/25] Undoing previous commit. --- tests/test_make_caster_adl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_make_caster_adl.cpp b/tests/test_make_caster_adl.cpp index c68a8d66ba..e854541390 100644 --- a/tests/test_make_caster_adl.cpp +++ b/tests/test_make_caster_adl.cpp @@ -7,7 +7,7 @@ struct type_mock {}; struct mock_caster { static int num() { return 101; } }; -mock_caster pybind11_select_caster(type_mock *, mock_caster * = nullptr); +mock_caster pybind11_select_caster(type_mock *); } // namespace adl_mock namespace adl_mrc { // minimal real caster @@ -41,7 +41,7 @@ struct minimal_real_caster { } }; -minimal_real_caster pybind11_select_caster(type_mrc *, minimal_real_caster * = nullptr); +minimal_real_caster pybind11_select_caster(type_mrc *); } // namespace adl_mrc From bc4a24130d250666482f3a9eb1885e360180b23e Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 2 Feb 2022 11:05:58 -0800 Subject: [PATCH 20/25] Using workaround only compilers that need it. --- include/pybind11/cast.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index bb3cbf3722..b565ddda8e 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -34,6 +34,11 @@ template class type_caster : public type template type_caster 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 struct make_caster_impl; @@ -48,6 +53,13 @@ struct make_caster_impl using make_caster = make_caster_impl>; +#else + +template +using make_caster = decltype(pybind11_select_caster(static_cast *>(nullptr))); + +#endif + // Shortcut for calling a caster's `cast_op_type` cast operator for casting a type_caster to a T template typename make_caster::template cast_op_type cast_op(make_caster &caster) { return caster.operator typename make_caster::template cast_op_type(); From 6f11212728e581c9dea2cfa1a1b37f2ddcecd4ac Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 2 Feb 2022 12:06:41 -0800 Subject: [PATCH 21/25] Fixing silly oversight (missing parentheses). --- include/pybind11/cast.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index b565ddda8e..8346689b79 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -37,7 +37,7 @@ template type_caster pybind11_select_cas // 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)) +#if defined(_MSC_VER) && (_MSC_VER < 1910 || (_MSC_VER < 1920 && !defined(PYBIND11_CPP17))) template struct make_caster_impl; From 98658773c3da145e06f4d00a7b5da3aebfc6b131 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 2 Feb 2022 12:10:39 -0800 Subject: [PATCH 22/25] Using enable_if_t to make the workaround code a little more compact. --- include/pybind11/cast.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 8346689b79..e3615141a3 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -43,11 +43,11 @@ template struct make_caster_impl; template -struct make_caster_impl::value>::type> +struct make_caster_impl::value>> : type_caster {}; template -struct make_caster_impl::value>::type> +struct make_caster_impl::value>> : decltype(pybind11_select_caster(static_cast(nullptr))) {}; template From 4574aba63f3e3d44642e9bdf91224fd3685e7183 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 2 Feb 2022 13:22:31 -0800 Subject: [PATCH 23/25] Additional mock_caster testing for global & unnamed namespaces. --- tests/test_make_caster_adl.cpp | 49 +++++++++++++++++++++++++--------- tests/test_make_caster_adl.py | 14 +++++----- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/tests/test_make_caster_adl.cpp b/tests/test_make_caster_adl.cpp index e854541390..44141d2a5e 100644 --- a/tests/test_make_caster_adl.cpp +++ b/tests/test_make_caster_adl.cpp @@ -1,16 +1,39 @@ -#include "pybind11_tests.h" - // adl = Argument Dependent Lookup -namespace adl_mock { +#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 adl_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; } +}; +#if defined(__GNUC__) +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wunneeded-internal-declaration" +#endif +unnamed_ns_mock_caster pybind11_select_caster(unnamed_ns_type_mock *); +#if defined(__GNUC__) +# pragma GCC diagnostic pop +#endif +} // namespace -namespace adl_mrc { // minimal real caster +namespace mrc_ns { // minimal real caster struct type_mrc { int value = -9999; @@ -31,7 +54,7 @@ struct minimal_real_caster { // NOLINTNEXTLINE(google-explicit-constructor) operator type_mrc const &() { static type_mrc obj; - obj.value = 303; + obj.value = 404; return obj; } @@ -43,15 +66,17 @@ struct minimal_real_caster { minimal_real_caster pybind11_select_caster(type_mrc *); -} // namespace adl_mrc +} // namespace mrc_ns TEST_SUBMODULE(make_caster_adl, m) { - m.def("num_mock", []() { return py::detail::make_caster::num(); }); + m.def("have_a_ns_num", []() { return py::detail::make_caster::num(); }); + m.def("global_ns_num", []() { return py::detail::make_caster::num(); }); + m.def("unnamed_ns_num", []() { return py::detail::make_caster::num(); }); - m.def("obj_mrc_return", []() { - adl_mrc::type_mrc obj; - obj.value = 404; + m.def("mrc_return", []() { + mrc_ns::type_mrc obj; + obj.value = 505; return obj; }); - m.def("obj_mrc_arg", [](adl_mrc::type_mrc const &obj) { return obj.value + 2000; }); + m.def("mrc_arg", [](mrc_ns::type_mrc const &obj) { return obj.value + 2000; }); } diff --git a/tests/test_make_caster_adl.py b/tests/test_make_caster_adl.py index d43d00fe42..9c77409dde 100644 --- a/tests/test_make_caster_adl.py +++ b/tests/test_make_caster_adl.py @@ -4,13 +4,15 @@ from pybind11_tests import make_caster_adl as m -def test_mock_caster(): - assert m.num_mock() == 101 +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_minimal_real_caster(): - assert m.obj_mrc_return() == 1404 - assert m.obj_mrc_arg(u"ignored") == 2303 + assert m.mrc_return() == 1505 + assert m.mrc_arg(u"ignored") == 2404 with pytest.raises(TypeError) as excinfo: - m.obj_mrc_arg(None) - assert "(arg0: adl_mrc::type_mrc) -> int" in str(excinfo.value) + m.mrc_arg(None) + assert "(arg0: mrc_ns::type_mrc) -> int" in str(excinfo.value) From dfd923d74324a72bf2aacf5ec146cbac6312b920 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 2 Feb 2022 15:45:49 -0800 Subject: [PATCH 24/25] Replacing non-portable warning-suppression pragma with a dummy implementation. --- tests/test_make_caster_adl.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/test_make_caster_adl.cpp b/tests/test_make_caster_adl.cpp index 44141d2a5e..0fe66b62c9 100644 --- a/tests/test_make_caster_adl.cpp +++ b/tests/test_make_caster_adl.cpp @@ -23,13 +23,15 @@ struct unnamed_ns_type_mock {}; struct unnamed_ns_mock_caster { static int num() { return 303; } }; -#if defined(__GNUC__) -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wunneeded-internal-declaration" -#endif unnamed_ns_mock_caster pybind11_select_caster(unnamed_ns_type_mock *); -#if defined(__GNUC__) -# pragma GCC diagnostic pop +#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 @@ -79,4 +81,9 @@ TEST_SUBMODULE(make_caster_adl, m) { 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(nullptr)); +#endif } From 59c8c772930acd15462ea9a0587995dde0d12bd4 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 6 Feb 2022 08:41:27 -0800 Subject: [PATCH 25/25] Adding test_make_caster_adl_alt --- tests/CMakeLists.txt | 1 + tests/test_make_caster_adl.py | 5 +++++ tests/test_make_caster_adl_alt.cpp | 13 +++++++++++++ tests/test_make_caster_adl_alt.py | 7 +++++++ 4 files changed, 26 insertions(+) create mode 100644 tests/test_make_caster_adl_alt.cpp create mode 100644 tests/test_make_caster_adl_alt.py diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d2417de944..2ff03ff839 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -138,6 +138,7 @@ set(PYBIND11_TEST_FILES 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.py b/tests/test_make_caster_adl.py index 9c77409dde..d420607cf0 100644 --- a/tests/test_make_caster_adl.py +++ b/tests/test_make_caster_adl.py @@ -2,6 +2,7 @@ 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(): @@ -10,6 +11,10 @@ def test_mock_casters(): 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 diff --git a/tests/test_make_caster_adl_alt.cpp b/tests/test_make_caster_adl_alt.cpp new file mode 100644 index 0000000000..1feb5be50e --- /dev/null +++ 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::num(); }); +} diff --git a/tests/test_make_caster_adl_alt.py b/tests/test_make_caster_adl_alt.py new file mode 100644 index 0000000000..d66093e589 --- /dev/null +++ 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