From a406a62e5b3ca74a76b6db9a1cd7cf3578c50772 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 24 Jun 2024 08:59:55 -0700 Subject: [PATCH] fix: use `std::addressof` in type_caster_base.h (#5189) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add tests only. Fails to build with this error message: ``` g++ -o pybind11/tests/test_copy_move.os -c -std=c++17 -fPIC -fvisibility=hidden -O0 -g -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -Wundef -Wnon-virtual-dtor -Wunused-result -Werror -isystem /usr/include/python3.11 -isystem /usr/include/eigen3 -DPYBIND11_STRICT_ASSERTS_CLASS_HOLDER_VS_TYPE_CASTER_MIX -DPYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE -DPYBIND11_TEST_BOOST -DPYBIND11_INTERNALS_VERSION=10000000 -Ipybind11/include -I/usr/local/google/home/rwgk/forked/pybind11/include -I/usr/local/google/home/rwgk/clone/pybind11/include /usr/local/google/home/rwgk/forked/pybind11/tests/test_copy_move.cpp In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../cast.h:15, from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../attr.h:14, from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/class.h:12, from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:13, from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/stl.h:12, from /usr/local/google/home/rwgk/forked/pybind11/tests/test_copy_move.cpp:11: /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h: In instantiation of ‘static pybind11::handle pybind11::detail::type_caster_base::cast(const itype&, pybind11::return_value_policy, pybind11::handle) [with type = UnusualOpRef; itype = UnusualOpRef]’: /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../cast.h:1230:37: required from ‘pybind11::object pybind11::cast(T&&, return_value_policy, handle) [with T = const UnusualOpRef&; typename std::enable_if<(! std::is_base_of::type>::value), int>::type = 0]’ /usr/local/google/home/rwgk/forked/pybind11/tests/test_copy_move.cpp:162:80: required from here /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1110:20: error: no matching function for call to ‘pybind11::detail::type_caster_base::cast(const UnusualOpRef::NonTrivialType, pybind11::return_value_policy&, pybind11::handle&)’ 1110 | return cast(&src, policy, parent); | ~~~~^~~~~~~~~~~~~~~~~~~~~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1105:19: note: candidate: ‘static pybind11::handle pybind11::detail::type_caster_base::cast(const itype&, pybind11::return_value_policy, pybind11::handle) [with type = UnusualOpRef; itype = UnusualOpRef]’ 1105 | static handle cast(const itype &src, return_value_policy policy, handle parent) { | ^~~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1105:37: note: no known conversion for argument 1 from ‘const UnusualOpRef::NonTrivialType’ {aka ‘const std::shared_ptr’} to ‘const pybind11::detail::type_caster_base::itype&’ {aka ‘const UnusualOpRef&’} 1105 | static handle cast(const itype &src, return_value_policy policy, handle parent) { | ~~~~~~~~~~~~~^~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1113:19: note: candidate: ‘static pybind11::handle pybind11::detail::type_caster_base::cast(itype&&, pybind11::return_value_policy, pybind11::handle) [with type = UnusualOpRef; itype = UnusualOpRef]’ 1113 | static handle cast(itype &&src, return_value_policy, handle parent) { | ^~~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1113:32: note: no known conversion for argument 1 from ‘const UnusualOpRef::NonTrivialType’ {aka ‘const std::shared_ptr’} to ‘pybind11::detail::type_caster_base::itype&&’ {aka ‘UnusualOpRef&&’} 1113 | static handle cast(itype &&src, return_value_policy, handle parent) { | ~~~~~~~~^~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1142:19: note: candidate: ‘static pybind11::handle pybind11::detail::type_caster_base::cast(const itype*, pybind11::return_value_policy, pybind11::handle) [with type = UnusualOpRef; itype = UnusualOpRef]’ 1142 | static handle cast(const itype *src, return_value_policy policy, handle parent) { | ^~~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1142:37: note: no known conversion for argument 1 from ‘const UnusualOpRef::NonTrivialType’ {aka ‘const std::shared_ptr’} to ‘const pybind11::detail::type_caster_base::itype*’ {aka ‘const UnusualOpRef*’} 1142 | static handle cast(const itype *src, return_value_policy policy, handle parent) { | ~~~~~~~~~~~~~^~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h: In instantiation of ‘static pybind11::handle pybind11::detail::type_caster_base::cast(itype&&, pybind11::return_value_policy, pybind11::handle) [with type = UnusualOpRef; itype = UnusualOpRef]’: /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../cast.h:1230:37: required from ‘pybind11::object pybind11::cast(T&&, return_value_policy, handle) [with T = UnusualOpRef; typename std::enable_if<(! std::is_base_of::type>::value), int>::type = 0]’ /usr/local/google/home/rwgk/forked/pybind11/tests/test_copy_move.cpp:163:74: required from here /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1114:20: error: no matching function for call to ‘pybind11::detail::type_caster_base::cast(UnusualOpRef::NonTrivialType, pybind11::return_value_policy, pybind11::handle&)’ 1114 | return cast(&src, return_value_policy::move, parent); | ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1105:19: note: candidate: ‘static pybind11::handle pybind11::detail::type_caster_base::cast(const itype&, pybind11::return_value_policy, pybind11::handle) [with type = UnusualOpRef; itype = UnusualOpRef]’ 1105 | static handle cast(const itype &src, return_value_policy policy, handle parent) { | ^~~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1105:37: note: no known conversion for argument 1 from ‘UnusualOpRef::NonTrivialType’ {aka ‘std::shared_ptr’} to ‘const pybind11::detail::type_caster_base::itype&’ {aka ‘const UnusualOpRef&’} 1105 | static handle cast(const itype &src, return_value_policy policy, handle parent) { | ~~~~~~~~~~~~~^~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1113:19: note: candidate: ‘static pybind11::handle pybind11::detail::type_caster_base::cast(itype&&, pybind11::return_value_policy, pybind11::handle) [with type = UnusualOpRef; itype = UnusualOpRef]’ 1113 | static handle cast(itype &&src, return_value_policy, handle parent) { | ^~~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1113:32: note: no known conversion for argument 1 from ‘UnusualOpRef::NonTrivialType’ {aka ‘std::shared_ptr’} to ‘pybind11::detail::type_caster_base::itype&&’ {aka ‘UnusualOpRef&&’} 1113 | static handle cast(itype &&src, return_value_policy, handle parent) { | ~~~~~~~~^~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1142:19: note: candidate: ‘static pybind11::handle pybind11::detail::type_caster_base::cast(const itype*, pybind11::return_value_policy, pybind11::handle) [with type = UnusualOpRef; itype = UnusualOpRef]’ 1142 | static handle cast(const itype *src, return_value_policy policy, handle parent) { | ^~~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/../detail/type_caster_base.h:1142:37: note: no known conversion for argument 1 from ‘UnusualOpRef::NonTrivialType’ {aka ‘std::shared_ptr’} to ‘const pybind11::detail::type_caster_base::itype*’ {aka ‘const UnusualOpRef*’} 1142 | static handle cast(const itype *src, return_value_policy policy, handle parent) { | ~~~~~~~~~~~~~^~~ ``` * Replace `&src` with `std::addressof(src)` to fix the error building the added tests. * Fix accident (not sure how the `const` slipped in here when transferring the diff from pybind11k). --- include/pybind11/detail/type_caster_base.h | 4 ++-- tests/pybind11_tests.h | 13 +++++++++++++ tests/test_copy_move.cpp | 12 ++++++++++++ tests/test_copy_move.py | 6 ++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index f6234c3d2..fd8c81b9a 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -1107,11 +1107,11 @@ public: || policy == return_value_policy::automatic_reference) { policy = return_value_policy::copy; } - return cast(&src, policy, parent); + return cast(std::addressof(src), policy, parent); } static handle cast(itype &&src, return_value_policy, handle parent) { - return cast(&src, return_value_policy::move, parent); + return cast(std::addressof(src), return_value_policy::move, parent); } // Returns a (pointer, type_info) pair taking care of necessary type lookup for a diff --git a/tests/pybind11_tests.h b/tests/pybind11_tests.h index a7c00c2f9..7be58feb6 100644 --- a/tests/pybind11_tests.h +++ b/tests/pybind11_tests.h @@ -3,6 +3,8 @@ #include #include +#include + namespace py = pybind11; using namespace pybind11::literals; @@ -52,6 +54,17 @@ union IntFloat { float f; }; +class UnusualOpRef { +public: + using NonTrivialType = std::shared_ptr; // Almost any non-trivial type will do. + // Overriding operator& should not break pybind11. + NonTrivialType operator&() { return non_trivial_member; } + NonTrivialType operator&() const { return non_trivial_member; } + +private: + NonTrivialType non_trivial_member; +}; + /// Custom cast-only type that casts to a string "rvalue" or "lvalue" depending on the cast /// context. Used to test recursive casters (e.g. std::tuple, stl containers). struct RValueCaster {}; diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp index 4f93defe1..ddffbe3a9 100644 --- a/tests/test_copy_move.cpp +++ b/tests/test_copy_move.cpp @@ -157,6 +157,13 @@ public: PYBIND11_NAMESPACE_END(detail) PYBIND11_NAMESPACE_END(pybind11) +namespace { + +py::object CastUnusualOpRefConstRef(const UnusualOpRef &cref) { return py::cast(cref); } +py::object CastUnusualOpRefMovable(UnusualOpRef &&mvbl) { return py::cast(std::move(mvbl)); } + +} // namespace + TEST_SUBMODULE(copy_move_policies, m) { // test_lacking_copy_ctor py::class_(m, "lacking_copy_ctor") @@ -293,6 +300,11 @@ TEST_SUBMODULE(copy_move_policies, m) { // Make sure that cast from pytype rvalue to other pytype works m.def("get_pytype_rvalue_castissue", [](double i) { return py::float_(i).cast(); }); + + py::class_(m, "UnusualOpRef"); + m.def("CallCastUnusualOpRefConstRef", + []() { return CastUnusualOpRefConstRef(UnusualOpRef()); }); + m.def("CallCastUnusualOpRefMovable", []() { return CastUnusualOpRefMovable(UnusualOpRef()); }); } /* diff --git a/tests/test_copy_move.py b/tests/test_copy_move.py index 405e00132..ee046305f 100644 --- a/tests/test_copy_move.py +++ b/tests/test_copy_move.py @@ -132,3 +132,9 @@ def test_pytype_rvalue_cast(): value = m.get_pytype_rvalue_castissue(1.0) assert value == 1 + + +def test_unusual_op_ref(): + # Merely to test that this still exists and built successfully. + assert m.CallCastUnusualOpRefConstRef().__class__.__name__ == "UnusualOpRef" + assert m.CallCastUnusualOpRefMovable().__class__.__name__ == "UnusualOpRef"