Skip to content

Commit

Permalink
fix: use std::addressof in type_caster_base.h (#5189)
Browse files Browse the repository at this point in the history
* 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<type>::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<detail::pyobject_tag, typename std::remove_reference<_Tp>::type>::value), int>::type <anonymous> = 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<UnusualOpRef>::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<type>::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<int>’} to ‘const pybind11::detail::type_caster_base<UnusualOpRef>::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<type>::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<int>’} to ‘pybind11::detail::type_caster_base<UnusualOpRef>::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<type>::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<int>’} to ‘const pybind11::detail::type_caster_base<UnusualOpRef>::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<type>::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<detail::pyobject_tag, typename std::remove_reference<_Tp>::type>::value), int>::type <anonymous> = 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<UnusualOpRef>::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<type>::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<int>’} to ‘const pybind11::detail::type_caster_base<UnusualOpRef>::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<type>::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<int>’} to ‘pybind11::detail::type_caster_base<UnusualOpRef>::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<type>::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<int>’} to ‘const pybind11::detail::type_caster_base<UnusualOpRef>::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).
  • Loading branch information
Ralf W. Grosse-Kunstleve authored Jun 24, 2024
1 parent f1a2e03 commit a406a62
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 2 deletions.
4 changes: 2 additions & 2 deletions include/pybind11/detail/type_caster_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -1107,11 +1107,11 @@ class type_caster_base : public type_caster_generic {
|| 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
Expand Down
13 changes: 13 additions & 0 deletions tests/pybind11_tests.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <pybind11/eval.h>
#include <pybind11/pybind11.h>

#include <memory>

namespace py = pybind11;
using namespace pybind11::literals;

Expand Down Expand Up @@ -52,6 +54,17 @@ union IntFloat {
float f;
};

class UnusualOpRef {
public:
using NonTrivialType = std::shared_ptr<int>; // 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 {};
Expand Down
12 changes: 12 additions & 0 deletions tests/test_copy_move.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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::int_>(); });

py::class_<UnusualOpRef>(m, "UnusualOpRef");
m.def("CallCastUnusualOpRefConstRef",
[]() { return CastUnusualOpRefConstRef(UnusualOpRef()); });
m.def("CallCastUnusualOpRefMovable", []() { return CastUnusualOpRefMovable(UnusualOpRef()); });
}

/*
Expand Down
6 changes: 6 additions & 0 deletions tests/test_copy_move.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

0 comments on commit a406a62

Please sign in to comment.