Use std::addressof() in type_caster_base.h and smart_holder_type_casters.h (#30093)

* changes as transferred from CL

* Add minimal tests.

* Add `non_trivial_member` to make the test more realistic and avoid clang-tidy errors.

* Also cover `CastConstRef` (to be verified).

* `type_caster::cast(const &)` evidently does not need `std::addressof()`

* Slightly simplify `UnusualOpRef`

* Reorganize tests so that it is easy to backport the type_caster_base.h change plus test.

Author: rwgk

include/pybind11/detail/smart_holder_type_casters.h

@@ -746,7 +746,7 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
 
     static handle cast(T &&src, return_value_policy /*policy*/, handle parent) {
         // type_caster_base BEGIN
-        return cast(&src, return_value_policy::move, parent);
+        return cast(std::addressof(src), return_value_policy::move, parent);
         // type_caster_base END
     }
 

include/pybind11/detail/type_caster_base.h

@@ -1120,7 +1120,7 @@ class type_caster_base : public type_caster_generic {
     }
 
     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

tests/pybind11_tests.h

@@ -3,6 +3,8 @@
 #include <pybind11/eval.h>
 #include <pybind11/pybind11.h>
 
+#include <memory>
+
 namespace py = pybind11;
 using namespace pybind11::literals;
 
@@ -52,6 +54,15 @@ union IntFloat {
     float f;
 };
 
+class UnusualOpRef {
+public:
+    using NonTrivialType = std::shared_ptr<int>; // Almost any non-trivial type will do.
+    NonTrivialType operator&() { return non_trivial_member; } // UNUSUAL operator.
+
+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 {};

tests/test_class_sh_basic.cpp

@@ -129,12 +129,17 @@ struct SharedPtrStash {
     void Add(const std::shared_ptr<const atyp> &obj) { stash.push_back(obj); }
 };
 
+class LocalUnusualOpRef : UnusualOpRef {}; // To avoid clashing with `py::class_<UnusualOpRef>`.
+py::object CastUnusualOpRefConstRef(const LocalUnusualOpRef &cref) { return py::cast(cref); }
+py::object CastUnusualOpRefMovable(LocalUnusualOpRef &&mvbl) { return py::cast(std::move(mvbl)); }
+
 } // namespace class_sh_basic
 } // namespace pybind11_tests
 
 PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_basic::atyp)
 PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_basic::uconsumer)
 PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_basic::SharedPtrStash)
+PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_basic::LocalUnusualOpRef)
 
 namespace pybind11_tests {
 namespace class_sh_basic {
@@ -227,6 +232,12 @@ TEST_SUBMODULE(class_sh_basic, m) {
         "rtrn_uq_automatic_reference",
         []() { return std::unique_ptr<atyp>(new atyp("rtrn_uq_automatic_reference")); },
         pybind11::return_value_policy::automatic_reference);
+
+    py::classh<LocalUnusualOpRef>(m, "LocalUnusualOpRef");
+    m.def("CallCastUnusualOpRefConstRef",
+          []() { return CastUnusualOpRefConstRef(LocalUnusualOpRef()); });
+    m.def("CallCastUnusualOpRefMovable",
+          []() { return CastUnusualOpRefMovable(LocalUnusualOpRef()); });
 }
 
 } // namespace class_sh_basic

tests/test_class_sh_basic.py

@@ -216,3 +216,9 @@ def test_function_signatures(doc):
 
 def test_unique_ptr_return_value_policy_automatic_reference():
     assert m.get_mtxt(m.rtrn_uq_automatic_reference()) == "rtrn_uq_automatic_reference"
+
+
+def test_unusual_op_ref():
+    # Merely to test that this still exists and built successfully.
+    assert m.CallCastUnusualOpRefConstRef().__class__.__name__ == "LocalUnusualOpRef"
+    assert m.CallCastUnusualOpRefMovable().__class__.__name__ == "LocalUnusualOpRef"

tests/test_copy_move.cpp

@@ -157,6 +157,13 @@ struct type_caster<CopyOnlyInt> {
 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_<lacking_copy_ctor>(m, "lacking_copy_ctor")
@@ -295,6 +302,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::int_>(); });
 
+    py::class_<UnusualOpRef>(m, "UnusualOpRef");
+    m.def("CallCastUnusualOpRefConstRef",
+          []() { return CastUnusualOpRefConstRef(UnusualOpRef()); });
+    m.def("CallCastUnusualOpRefMovable", []() { return CastUnusualOpRefMovable(UnusualOpRef()); });
+
     // Mimic situation generated by PyCLIF-pybind11.
     // Requires `case return_value_policy::_clif_automatic` in `type_caster_base`.
     struct PyCastUsingClifAutomaticTestType {};

tests/test_copy_move.py

@@ -132,6 +132,12 @@ def test_pytype_rvalue_cast():
     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"
+
+
 def test_py_cast_using_clif_automatic():
     obj = m.py_cast_using_clif_automatic()
     assert obj.__class__.__name__ == "PyCastUsingClifAutomaticTestType"