Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: __getattr__ and smart_holders #3788

Closed
3 tasks done
petersteneteg opened this issue Mar 9, 2022 · 8 comments
Closed
3 tasks done

[BUG]: __getattr__ and smart_holders #3788

petersteneteg opened this issue Mar 9, 2022 · 8 comments
Assignees
Labels
triage New bug, unverified

Comments

@petersteneteg
Copy link

Required prerequisites

Problem description

This is a problem in the smart_holder branch

When defining __getattr__ in the bindings for a class (see example code below) using the master pybind11 branch I can do the following:

In [1]: import foo
In [2]: f = foo.Foo()
In [3]: f.bar()
Out[3]: 42
In [5]: f.something
Out[5]: 'GetAttr: something'

and everything works as expected.

But when using the smart_holder branch and defining PYBIND11_USE_SMART_HOLDER_AS_DEFAULT
either calling f.bar() or f.something results in infinite recursion, hanging the runtime. Using "conservative mode" gives the same result.

Im not sure if I'm doing anything bad, and I can not find anything about it not being supported it in the documentation, and it works perfectly fine in the master branch.

Reproducible example code

#include <pybind11/pybind11.h>

struct Foo {
    Foo() = default;
    int bar() const { return 42; }
};

namespace py = pybind11;

PYBIND11_MODULE(foo, m) {
    py::class_<Foo>(m, "Foo")
        .def(py::init<>())
        .def("bar", &Foo::bar)
        .def("__getattr__", [](Foo&, std::string key) { return "GetAttr: " + key; });
}
@petersteneteg petersteneteg added the triage New bug, unverified label Mar 9, 2022
@Skylion007 Skylion007 assigned Skylion007 and rwgk and unassigned Skylion007 Mar 9, 2022
@petersteneteg
Copy link
Author

Btw, just using the smart_holder without defining PYBIND11_USE_SMART_HOLDER_AS_DEFAULT or using py::classy works just fine.

@petersteneteg
Copy link
Author

The recursive call stack I get looks like this:

    .
    .
    .
    frame #16945: 0x0000000100e8fb44 foo.cpython-310-darwin.so`pybind11::cpp_function::dispatcher(self=0x0000000100c36760, args_in=0x0000000100c18300, kwargs_in=0x0000000000000000) at pybind11.h:925:30
    frame #16946: 0x000000010060817c Python`cfunction_call + 60
    frame #16947: 0x00000001005b594c Python`_PyObject_MakeTpCall + 136
    frame #16948: 0x00000001005b8da8 Python`method_vectorcall + 604
    frame #16949: 0x00000001006313f4 Python`call_attribute + 152
    frame #16950: 0x000000010062b638 Python`slot_tp_getattr_hook + 436
    frame #16951: 0x000000010060ee98 Python`PyObject_GetAttrString + 116
    frame #16952: 0x000000010060f424 Python`PyObject_HasAttrString + 16
    frame #16953: 0x0000000100e90aa4 foo.cpython-310-darwin.so`pybind11::hasattr(obj=handle @ 0x000000016fdfbcf8, name="as_Foo") at pytypes.h:518:12
    frame #16954: 0x0000000100eb50cc foo.cpython-310-darwin.so`pybind11::detail::modified_type_caster_generic_load_impl::try_as_void_ptr_capsule(this=0x000000016fdfc270, src=handle @ 0x000000016fdfbdd8) at smart_holder_type_casters.h:131:13
    frame #16955: 0x0000000100eb4ab0 foo.cpython-310-darwin.so`bool pybind11::detail::modified_type_caster_generic_load_impl::load_impl<pybind11::detail::modified_type_caster_generic_load_impl>(this=0x000000016fdfc270, src=handle @ 0x000000016fdfc010, convert=true) at smart_holder_type_casters.h:203:24
    frame #16956: 0x0000000100eb45e0 foo.cpython-310-darwin.so`pybind11::detail::modified_type_caster_generic_load_impl::load(this=0x000000016fdfc270, src=handle @ 0x000000016fdfc058, convert=true) at smart_holder_type_casters.h:62:16
    frame #16957: 0x0000000100ebbd08 foo.cpython-310-darwin.so`pybind11::detail::smart_holder_type_caster_load<Foo>::load(this=0x000000016fdfc270, src=handle @ 0x000000016fdfc110, convert=true) at smart_holder_type_casters.h:416:24
    frame #16958: 0x0000000100ebcdb0 foo.cpython-310-darwin.so`bool pybind11::detail::argument_loader<Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >::load_impl_sequence<0ul, 1ul>(this=0x000000016fdfc270, call=0x000000016fdfc9b0, (null)=std::__1::index_sequence<0UL, 1UL> @ 0x000000016fdfc1ae) at cast.h:1427:47
    frame #16959: 0x0000000100ebcb64 foo.cpython-310-darwin.so`pybind11::detail::argument_loader<Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >::load_args(this=0x000000016fdfc270, call=0x000000016fdfc9b0) at cast.h:1405:50
    frame #16960: 0x0000000100ebc9ec foo.cpython-310-darwin.so`void pybind11::cpp_function::initialize<pybind11_init_foo(pybind11::module_&)::$_0, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, pybind11::name, pybind11::is_method, pybind11::sibling>(this=0x000000016fdfc9b0, call=0x000000016fdfc9b0)::$_0&&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (*)(Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >), pybind11::name const&, pybind11::is_method const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_call&)::operator()(pybind11::detail::function_call&) const at pybind11.h:229:33
    frame #16961: 0x0000000100ebc9a0 foo.cpython-310-darwin.so`void pybind11::cpp_function::initialize<pybind11_init_foo(pybind11::module_&)::$_0, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, pybind11::name, pybind11::is_method, pybind11::sibling>(call=0x000000016fdfc9b0)::$_0&&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (*)(Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >), pybind11::name const&, pybind11::is_method const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_call&)::__invoke(pybind11::detail::function_call&) at pybind11.h:225:21
    frame #16962: 0x0000000100e8fb44 foo.cpython-310-darwin.so`pybind11::cpp_function::dispatcher(self=0x0000000100c36760, args_in=0x0000000100c4a640, kwargs_in=0x0000000000000000) at pybind11.h:925:30
    frame #16963: 0x000000010060817c Python`cfunction_call + 60
    frame #16964: 0x00000001005b594c Python`_PyObject_MakeTpCall + 136
    frame #16965: 0x00000001005b8da8 Python`method_vectorcall + 604
    frame #16966: 0x00000001006313f4 Python`call_attribute + 152
    frame #16967: 0x000000010062b638 Python`slot_tp_getattr_hook + 436
    frame #16968: 0x000000010060ee98 Python`PyObject_GetAttrString + 116
    frame #16969: 0x000000010060f424 Python`PyObject_HasAttrString + 16
    frame #16970: 0x0000000100e90aa4 foo.cpython-310-darwin.so`pybind11::hasattr(obj=handle @ 0x000000016fdfcda8, name="as_Foo") at pytypes.h:518:12
    frame #16971: 0x0000000100eb50cc foo.cpython-310-darwin.so`pybind11::detail::modified_type_caster_generic_load_impl::try_as_void_ptr_capsule(this=0x000000016fdfd320, src=handle @ 0x000000016fdfce88) at smart_holder_type_casters.h:131:13
    frame #16972: 0x0000000100eb4ab0 foo.cpython-310-darwin.so`bool pybind11::detail::modified_type_caster_generic_load_impl::load_impl<pybind11::detail::modified_type_caster_generic_load_impl>(this=0x000000016fdfd320, src=handle @ 0x000000016fdfd0c0, convert=true) at smart_holder_type_casters.h:203:24
    frame #16973: 0x0000000100eb45e0 foo.cpython-310-darwin.so`pybind11::detail::modified_type_caster_generic_load_impl::load(this=0x000000016fdfd320, src=handle @ 0x000000016fdfd108, convert=true) at smart_holder_type_casters.h:62:16
    frame #16974: 0x0000000100ebbd08 foo.cpython-310-darwin.so`pybind11::detail::smart_holder_type_caster_load<Foo>::load(this=0x000000016fdfd320, src=handle @ 0x000000016fdfd1c0, convert=true) at smart_holder_type_casters.h:416:24
    frame #16975: 0x0000000100ebcdb0 foo.cpython-310-darwin.so`bool pybind11::detail::argument_loader<Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >::load_impl_sequence<0ul, 1ul>(this=0x000000016fdfd320, call=0x000000016fdfda60, (null)=std::__1::index_sequence<0UL, 1UL> @ 0x000000016fdfd25e) at cast.h:1427:47
    frame #16976: 0x0000000100ebcb64 foo.cpython-310-darwin.so`pybind11::detail::argument_loader<Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >::load_args(this=0x000000016fdfd320, call=0x000000016fdfda60) at cast.h:1405:50
    frame #16977: 0x0000000100ebc9ec foo.cpython-310-darwin.so`void pybind11::cpp_function::initialize<pybind11_init_foo(pybind11::module_&)::$_0, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, pybind11::name, pybind11::is_method, pybind11::sibling>(this=0x000000016fdfda60, call=0x000000016fdfda60)::$_0&&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (*)(Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >), pybind11::name const&, pybind11::is_method const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_call&)::operator()(pybind11::detail::function_call&) const at pybind11.h:229:33
    frame #16978: 0x0000000100ebc9a0 foo.cpython-310-darwin.so`void pybind11::cpp_function::initialize<pybind11_init_foo(pybind11::module_&)::$_0, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, pybind11::name, pybind11::is_method, pybind11::sibling>(call=0x000000016fdfda60)::$_0&&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (*)(Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >), pybind11::name const&, pybind11::is_method const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_call&)::__invoke(pybind11::detail::function_call&) at pybind11.h:225:21
    frame #16979: 0x0000000100e8fb44 foo.cpython-310-darwin.so`pybind11::cpp_function::dispatcher(self=0x0000000100c36760, args_in=0x0000000100c18780, kwargs_in=0x0000000000000000) at pybind11.h:925:30
    frame #16980: 0x000000010060817c Python`cfunction_call + 60
    frame #16981: 0x00000001005b594c Python`_PyObject_MakeTpCall + 136
    frame #16982: 0x00000001005b8da8 Python`method_vectorcall + 604
    frame #16983: 0x00000001006313f4 Python`call_attribute + 152
    frame #16984: 0x000000010062b638 Python`slot_tp_getattr_hook + 436
    frame #16985: 0x000000010060ee98 Python`PyObject_GetAttrString + 116
    frame #16986: 0x000000010060f424 Python`PyObject_HasAttrString + 16
    frame #16987: 0x0000000100e90aa4 foo.cpython-310-darwin.so`pybind11::hasattr(obj=handle @ 0x000000016fdfde58, name="as_Foo") at pytypes.h:518:12
    frame #16988: 0x0000000100eb50cc foo.cpython-310-darwin.so`pybind11::detail::modified_type_caster_generic_load_impl::try_as_void_ptr_capsule(this=0x000000016fdfe3d0, src=handle @ 0x000000016fdfdf38) at smart_holder_type_casters.h:131:13
    frame #16989: 0x0000000100eb4ab0 foo.cpython-310-darwin.so`bool pybind11::detail::modified_type_caster_generic_load_impl::load_impl<pybind11::detail::modified_type_caster_generic_load_impl>(this=0x000000016fdfe3d0, src=handle @ 0x000000016fdfe170, convert=true) at smart_holder_type_casters.h:203:24
    frame #16990: 0x0000000100eb45e0 foo.cpython-310-darwin.so`pybind11::detail::modified_type_caster_generic_load_impl::load(this=0x000000016fdfe3d0, src=handle @ 0x000000016fdfe1b8, convert=true) at smart_holder_type_casters.h:62:16
    frame #16991: 0x0000000100ebbd08 foo.cpython-310-darwin.so`pybind11::detail::smart_holder_type_caster_load<Foo>::load(this=0x000000016fdfe3d0, src=handle @ 0x000000016fdfe270, convert=true) at smart_holder_type_casters.h:416:24
    frame #16992: 0x0000000100ebcdb0 foo.cpython-310-darwin.so`bool pybind11::detail::argument_loader<Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >::load_impl_sequence<0ul, 1ul>(this=0x000000016fdfe3d0, call=0x000000016fdfeb10, (null)=std::__1::index_sequence<0UL, 1UL> @ 0x000000016fdfe30e) at cast.h:1427:47
    frame #16993: 0x0000000100ebcb64 foo.cpython-310-darwin.so`pybind11::detail::argument_loader<Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >::load_args(this=0x000000016fdfe3d0, call=0x000000016fdfeb10) at cast.h:1405:50
    frame #16994: 0x0000000100ebc9ec foo.cpython-310-darwin.so`void pybind11::cpp_function::initialize<pybind11_init_foo(pybind11::module_&)::$_0, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, pybind11::name, pybind11::is_method, pybind11::sibling>(this=0x000000016fdfeb10, call=0x000000016fdfeb10)::$_0&&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (*)(Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >), pybind11::name const&, pybind11::is_method const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_call&)::operator()(pybind11::detail::function_call&) const at pybind11.h:229:33
    frame #16995: 0x0000000100ebc9a0 foo.cpython-310-darwin.so`void pybind11::cpp_function::initialize<pybind11_init_foo(pybind11::module_&)::$_0, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, pybind11::name, pybind11::is_method, pybind11::sibling>(call=0x000000016fdfeb10)::$_0&&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > (*)(Foo&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >), pybind11::name const&, pybind11::is_method const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_call&)::__invoke(pybind11::detail::function_call&) at pybind11.h:225:21
    frame #16996: 0x0000000100e8fb44 foo.cpython-310-darwin.so`pybind11::cpp_function::dispatcher(self=0x0000000100c36760, args_in=0x0000000100c180c0, kwargs_in=0x0000000000000000) at pybind11.h:925:30
    frame #16997: 0x000000010060817c Python`cfunction_call + 60
    frame #16998: 0x00000001005b594c Python`_PyObject_MakeTpCall + 136
    frame #16999: 0x00000001005b8da8 Python`method_vectorcall + 604
    frame #17000: 0x00000001006313f4 Python`call_attribute + 152
    frame #17001: 0x000000010062b638 Python`slot_tp_getattr_hook + 436
    frame #17002: 0x00000001006a9710 Python`_PyEval_EvalFrameDefault + 21328
    frame #17003: 0x00000001006a30ec Python`_PyEval_Vector + 328
    frame #17004: 0x00000001006a2f90 Python`PyEval_EvalCode + 104
    frame #17005: 0x00000001006fc6cc Python`run_eval_code_obj + 84
    frame #17006: 0x00000001006fc614 Python`run_mod + 112
    frame #17007: 0x00000001006fc280 Python`pyrun_file + 148
    frame #17008: 0x00000001006fbb94 Python`_PyRun_SimpleFileObject + 268
    frame #17009: 0x00000001006fb1d4 Python`_PyRun_AnyFileObject + 232
    frame #17010: 0x000000010071d40c Python`pymain_run_file_obj + 220
    frame #17011: 0x000000010071cb5c Python`pymain_run_file + 72
    frame #17012: 0x000000010071c3cc Python`Py_RunMain + 868
    frame #17013: 0x000000010071d578 Python`pymain_main + 36
    frame #17014: 0x000000010071d7ec Python`Py_BytesMain + 40
    frame #17015: 0x00000001000150f4 dyld`start + 520

@petersteneteg
Copy link
Author

I think the problem occurs in on line

if (hasattr(src, as_void_ptr_function_name.c_str())) {

Which was added in PR #3633.

I think the hasattrcall on that line creates the recursive load problem that I'm seeing, with it interacts with the bound __getattr__ function of my class.
If I build with a commit from before #3633 everything works as expected.

@rwgk
Copy link
Collaborator

rwgk commented Mar 10, 2022

Thanks! I'm just in the middle of setting up a reproducer.

@rwgk
Copy link
Collaborator

rwgk commented Mar 10, 2022

@wangxf123456 for awareness.

rwgk pushed a commit to rwgk/pybind11 that referenced this issue Mar 10, 2022
Expected to build & run as-is. Uncommenting reproduces the infinite recursion.
rwgk pushed a commit that referenced this issue Mar 11, 2022
* Reproducer for #3788

Expected to build & run as-is. Uncommenting reproduces the infinite recursion.

* Moving try_as_void_ptr_capsule() to the end of load_impl()

* Moving new test into the existing test_class_sh_void_ptr_capsule

* Experiment

* Remove comments and simplify the test cases.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@rwgk
Copy link
Collaborator

rwgk commented Mar 11, 2022

Fixed!

@rwgk rwgk closed this as completed Mar 11, 2022
@rwgk
Copy link
Collaborator

rwgk commented Mar 11, 2022

(Where is the button to mark this as fixed?)

@petersteneteg
Copy link
Author

Awesome, that fixed it! Thanks for the quick solution!

rwgk pushed a commit to rwgk/pybind11 that referenced this issue Aug 30, 2024
rwgk pushed a commit to google/pybind11clif that referenced this issue Sep 1, 2024
…y between different independent Python/C++ bindings systems. (#30158)

* `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match.

* Include cleanup (mainly to resolve PyPy build failures).

* Fix clang-tidy errors.

* Resolve `error: extra

* factor out platform_abi_id.h from internals.h (no functional changes)

* factor out internals_version.h from internals.h (no functional changes)

* Update CMakeLists.txt, tests/extra_python_package/test_files.py

* Revert "factor out internals_version.h from internals.h (no functional changes)"

This reverts commit 3ccea8c.

* Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py

* `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name`

* Add PremiumTraveler

* Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h

* Expand tests: `PremiumTraveler`, `get_points()`

* Shuffle order of tests (no real changes).

* Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function.

* Use `type_caster_generic::load(self)` instead of `cast<T *>(self)`

* Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`).

* Make platform_abi_id.h completely stand-alone.

* rename exo_planet.cpp -> exo_planet_pybind11.cpp

* Add exo_planet_c_api.cpp (incomplete).

* Fix silly oversight (wrong filename in `#include`).

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); }
      | ~~~~~~           ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   14 | static PyMethodDef ThisMethodDef[]
      | ~~~~~~             ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   17 | static struct PyModuleDef ThisModuleDef = {
      | ~~~~~~                    ^
```

* Implement exo_planet_c_api GetLuggage(), GetPoints()

* Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()`

* Fix oversight.

* Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures.

* Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later.

* Small cleanup.

* Restore and add to `test_call_cpp_transporter_*()`

* Ensure pybind/pybind11#3788 does not bite again.

* `class_dunder_cpp_transporter()`: replace `obj.cast<std::string>()` with `std::string(obj)`

* Add (simple) copyright notices in all newly added files.

* Globally replace cpp_transporter with cpp_conduit

* style: pre-commit fixes

* IWYU fixes

* Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()`

* Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`.

This avoids the unicode decode/encode roundtrips:

* More robust (no decode/encode errors).

* Minor runtime optimization.

* Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes).

* Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change).

This provides an extra layer of protection against C++ ABI mismatches:

* The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions.

* The second layer is that the `typeid(std::type_info).name()`s must match between extensions.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
henryiii added a commit that referenced this issue Sep 13, 2024
…n/C++ bindings systems. (#5296)

* `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match.

* Include cleanup (mainly to resolve PyPy build failures).

* Fix clang-tidy errors.

* Resolve `error: extra

* factor out platform_abi_id.h from internals.h (no functional changes)

* factor out internals_version.h from internals.h (no functional changes)

* Update CMakeLists.txt, tests/extra_python_package/test_files.py

* Revert "factor out internals_version.h from internals.h (no functional changes)"

This reverts commit 3ccea8c.

* Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py

* `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name`

* Add PremiumTraveler

* Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h

* Expand tests: `PremiumTraveler`, `get_points()`

* Shuffle order of tests (no real changes).

* Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function.

* Use `type_caster_generic::load(self)` instead of `cast<T *>(self)`

* Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`).

* Make platform_abi_id.h completely stand-alone.

* rename exo_planet.cpp -> exo_planet_pybind11.cpp

* Add exo_planet_c_api.cpp (incomplete).

* Fix silly oversight (wrong filename in `#include`).

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); }
      | ~~~~~~           ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   14 | static PyMethodDef ThisMethodDef[]
      | ~~~~~~             ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   17 | static struct PyModuleDef ThisModuleDef = {
      | ~~~~~~                    ^
```

* Implement exo_planet_c_api GetLuggage(), GetPoints()

* Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()`

* Fix oversight.

* Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures.

* Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later.

* Small cleanup.

* Restore and add to `test_call_cpp_transporter_*()`

* Ensure #3788 does not bite again.

* `class_dunder_cpp_transporter()`: replace `obj.cast<std::string>()` with `std::string(obj)`

* Add (simple) copyright notices in all newly added files.

* Globally replace cpp_transporter with cpp_conduit

* style: pre-commit fixes

* IWYU fixes

* Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()`

* Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`.

This avoids the unicode decode/encode roundtrips:

* More robust (no decode/encode errors).

* Minor runtime optimization.

* Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes).

* Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change).

This provides an extra layer of protection against C++ ABI mismatches:

* The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions.

* The second layer is that the `typeid(std::type_info).name()`s must match between extensions.

* Fix sort order accident in tests/CMakeLists.txt

* Apply suggestions from code review

Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

* style: pre-commit fixes

* refactor: rename to _pybind_conduit_v1_

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

* Add test_home_planet_wrap_very_lonely_traveler(), test_exo_planet_pybind11_wrap_very_lonely_traveler()

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:39:32: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   10 |     py::class_<LonelyTraveler>(m, "LonelyTraveler");
      |                                ^
      |                                std::move( )
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:43:52: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   43 |     py::class_<VeryLonelyTraveler, LonelyTraveler>(m, "VeryLonelyTraveler");
      |                                                    ^
      |                                                    std::move( )
```

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>
rwgk added a commit to rwgk/pybind11 that referenced this issue Sep 13, 2024
…n/C++ bindings systems. (pybind#5296)

* `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match.

* Include cleanup (mainly to resolve PyPy build failures).

* Fix clang-tidy errors.

* Resolve `error: extra

* factor out platform_abi_id.h from internals.h (no functional changes)

* factor out internals_version.h from internals.h (no functional changes)

* Update CMakeLists.txt, tests/extra_python_package/test_files.py

* Revert "factor out internals_version.h from internals.h (no functional changes)"

This reverts commit 3ccea8c.

* Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py

* `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name`

* Add PremiumTraveler

* Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h

* Expand tests: `PremiumTraveler`, `get_points()`

* Shuffle order of tests (no real changes).

* Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function.

* Use `type_caster_generic::load(self)` instead of `cast<T *>(self)`

* Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`).

* Make platform_abi_id.h completely stand-alone.

* rename exo_planet.cpp -> exo_planet_pybind11.cpp

* Add exo_planet_c_api.cpp (incomplete).

* Fix silly oversight (wrong filename in `#include`).

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); }
      | ~~~~~~           ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   14 | static PyMethodDef ThisMethodDef[]
      | ~~~~~~             ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   17 | static struct PyModuleDef ThisModuleDef = {
      | ~~~~~~                    ^
```

* Implement exo_planet_c_api GetLuggage(), GetPoints()

* Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()`

* Fix oversight.

* Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures.

* Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later.

* Small cleanup.

* Restore and add to `test_call_cpp_transporter_*()`

* Ensure pybind#3788 does not bite again.

* `class_dunder_cpp_transporter()`: replace `obj.cast<std::string>()` with `std::string(obj)`

* Add (simple) copyright notices in all newly added files.

* Globally replace cpp_transporter with cpp_conduit

* style: pre-commit fixes

* IWYU fixes

* Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()`

* Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`.

This avoids the unicode decode/encode roundtrips:

* More robust (no decode/encode errors).

* Minor runtime optimization.

* Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes).

* Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change).

This provides an extra layer of protection against C++ ABI mismatches:

* The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions.

* The second layer is that the `typeid(std::type_info).name()`s must match between extensions.

* Fix sort order accident in tests/CMakeLists.txt

* Apply suggestions from code review

Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

* style: pre-commit fixes

* refactor: rename to _pybind_conduit_v1_

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

* Add test_home_planet_wrap_very_lonely_traveler(), test_exo_planet_pybind11_wrap_very_lonely_traveler()

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:39:32: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   10 |     py::class_<LonelyTraveler>(m, "LonelyTraveler");
      |                                ^
      |                                std::move( )
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:43:52: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   43 |     py::class_<VeryLonelyTraveler, LonelyTraveler>(m, "VeryLonelyTraveler");
      |                                                    ^
      |                                                    std::move( )
```

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>
rwgk added a commit to rwgk/pybind11 that referenced this issue Sep 13, 2024
…n/C++ bindings systems. (pybind#5296)

* `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match.

* Include cleanup (mainly to resolve PyPy build failures).

* Fix clang-tidy errors.

* Resolve `error: extra

* factor out platform_abi_id.h from internals.h (no functional changes)

* factor out internals_version.h from internals.h (no functional changes)

* Update CMakeLists.txt, tests/extra_python_package/test_files.py

* Revert "factor out internals_version.h from internals.h (no functional changes)"

This reverts commit 3ccea8c.

* Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py

* `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name`

* Add PremiumTraveler

* Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h

* Expand tests: `PremiumTraveler`, `get_points()`

* Shuffle order of tests (no real changes).

* Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function.

* Use `type_caster_generic::load(self)` instead of `cast<T *>(self)`

* Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`).

* Make platform_abi_id.h completely stand-alone.

* rename exo_planet.cpp -> exo_planet_pybind11.cpp

* Add exo_planet_c_api.cpp (incomplete).

* Fix silly oversight (wrong filename in `#include`).

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); }
      | ~~~~~~           ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   14 | static PyMethodDef ThisMethodDef[]
      | ~~~~~~             ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   17 | static struct PyModuleDef ThisModuleDef = {
      | ~~~~~~                    ^
```

* Implement exo_planet_c_api GetLuggage(), GetPoints()

* Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()`

* Fix oversight.

* Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures.

* Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later.

* Small cleanup.

* Restore and add to `test_call_cpp_transporter_*()`

* Ensure pybind#3788 does not bite again.

* `class_dunder_cpp_transporter()`: replace `obj.cast<std::string>()` with `std::string(obj)`

* Add (simple) copyright notices in all newly added files.

* Globally replace cpp_transporter with cpp_conduit

* style: pre-commit fixes

* IWYU fixes

* Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()`

* Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`.

This avoids the unicode decode/encode roundtrips:

* More robust (no decode/encode errors).

* Minor runtime optimization.

* Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes).

* Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change).

This provides an extra layer of protection against C++ ABI mismatches:

* The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions.

* The second layer is that the `typeid(std::type_info).name()`s must match between extensions.

* Fix sort order accident in tests/CMakeLists.txt

* Apply suggestions from code review

Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

* style: pre-commit fixes

* refactor: rename to _pybind_conduit_v1_

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

* Add test_home_planet_wrap_very_lonely_traveler(), test_exo_planet_pybind11_wrap_very_lonely_traveler()

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:39:32: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   10 |     py::class_<LonelyTraveler>(m, "LonelyTraveler");
      |                                ^
      |                                std::move( )
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:43:52: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   43 |     py::class_<VeryLonelyTraveler, LonelyTraveler>(m, "VeryLonelyTraveler");
      |                                                    ^
      |                                                    std::move( )
```

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>
henryiii added a commit that referenced this issue Sep 13, 2024
…n/C++ bindings systems. (#5296)

* `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match.

* Include cleanup (mainly to resolve PyPy build failures).

* Fix clang-tidy errors.

* Resolve `error: extra

* factor out platform_abi_id.h from internals.h (no functional changes)

* factor out internals_version.h from internals.h (no functional changes)

* Update CMakeLists.txt, tests/extra_python_package/test_files.py

* Revert "factor out internals_version.h from internals.h (no functional changes)"

This reverts commit 3ccea8c.

* Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py

* `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name`

* Add PremiumTraveler

* Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h

* Expand tests: `PremiumTraveler`, `get_points()`

* Shuffle order of tests (no real changes).

* Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function.

* Use `type_caster_generic::load(self)` instead of `cast<T *>(self)`

* Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`).

* Make platform_abi_id.h completely stand-alone.

* rename exo_planet.cpp -> exo_planet_pybind11.cpp

* Add exo_planet_c_api.cpp (incomplete).

* Fix silly oversight (wrong filename in `#include`).

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); }
      | ~~~~~~           ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   14 | static PyMethodDef ThisMethodDef[]
      | ~~~~~~             ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   17 | static struct PyModuleDef ThisModuleDef = {
      | ~~~~~~                    ^
```

* Implement exo_planet_c_api GetLuggage(), GetPoints()

* Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()`

* Fix oversight.

* Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures.

* Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later.

* Small cleanup.

* Restore and add to `test_call_cpp_transporter_*()`

* Ensure #3788 does not bite again.

* `class_dunder_cpp_transporter()`: replace `obj.cast<std::string>()` with `std::string(obj)`

* Add (simple) copyright notices in all newly added files.

* Globally replace cpp_transporter with cpp_conduit

* style: pre-commit fixes

* IWYU fixes

* Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()`

* Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`.

This avoids the unicode decode/encode roundtrips:

* More robust (no decode/encode errors).

* Minor runtime optimization.

* Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes).

* Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change).

This provides an extra layer of protection against C++ ABI mismatches:

* The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions.

* The second layer is that the `typeid(std::type_info).name()`s must match between extensions.

* Fix sort order accident in tests/CMakeLists.txt

* Apply suggestions from code review

Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

* style: pre-commit fixes

* refactor: rename to _pybind_conduit_v1_

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

* Add test_home_planet_wrap_very_lonely_traveler(), test_exo_planet_pybind11_wrap_very_lonely_traveler()

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:39:32: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   10 |     py::class_<LonelyTraveler>(m, "LonelyTraveler");
      |                                ^
      |                                std::move( )
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:43:52: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   43 |     py::class_<VeryLonelyTraveler, LonelyTraveler>(m, "VeryLonelyTraveler");
      |                                                    ^
      |                                                    std::move( )
```

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>
rwgk added a commit that referenced this issue Sep 13, 2024
… independent Python/C++ bindings systems. (#5368)

* Enable type-safe interoperability between different independent Python/C++ bindings systems. (#5296)

* `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match.

* Include cleanup (mainly to resolve PyPy build failures).

* Fix clang-tidy errors.

* Resolve `error: extra

* factor out platform_abi_id.h from internals.h (no functional changes)

* factor out internals_version.h from internals.h (no functional changes)

* Update CMakeLists.txt, tests/extra_python_package/test_files.py

* Revert "factor out internals_version.h from internals.h (no functional changes)"

This reverts commit 3ccea8c.

* Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py

* `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name`

* Add PremiumTraveler

* Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h

* Expand tests: `PremiumTraveler`, `get_points()`

* Shuffle order of tests (no real changes).

* Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function.

* Use `type_caster_generic::load(self)` instead of `cast<T *>(self)`

* Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`).

* Make platform_abi_id.h completely stand-alone.

* rename exo_planet.cpp -> exo_planet_pybind11.cpp

* Add exo_planet_c_api.cpp (incomplete).

* Fix silly oversight (wrong filename in `#include`).

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); }
      | ~~~~~~           ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   14 | static PyMethodDef ThisMethodDef[]
      | ~~~~~~             ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   17 | static struct PyModuleDef ThisModuleDef = {
      | ~~~~~~                    ^
```

* Implement exo_planet_c_api GetLuggage(), GetPoints()

* Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()`

* Fix oversight.

* Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures.

* Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later.

* Small cleanup.

* Restore and add to `test_call_cpp_transporter_*()`

* Ensure #3788 does not bite again.

* `class_dunder_cpp_transporter()`: replace `obj.cast<std::string>()` with `std::string(obj)`

* Add (simple) copyright notices in all newly added files.

* Globally replace cpp_transporter with cpp_conduit

* style: pre-commit fixes

* IWYU fixes

* Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()`

* Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`.

This avoids the unicode decode/encode roundtrips:

* More robust (no decode/encode errors).

* Minor runtime optimization.

* Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes).

* Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change).

This provides an extra layer of protection against C++ ABI mismatches:

* The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions.

* The second layer is that the `typeid(std::type_info).name()`s must match between extensions.

* Fix sort order accident in tests/CMakeLists.txt

* Apply suggestions from code review

Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

* style: pre-commit fixes

* refactor: rename to _pybind_conduit_v1_

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

* Add test_home_planet_wrap_very_lonely_traveler(), test_exo_planet_pybind11_wrap_very_lonely_traveler()

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:39:32: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   10 |     py::class_<LonelyTraveler>(m, "LonelyTraveler");
      |                                ^
      |                                std::move( )
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:43:52: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   43 |     py::class_<VeryLonelyTraveler, LonelyTraveler>(m, "VeryLonelyTraveler");
      |                                                    ^
      |                                                    std::move( )
```

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

* Remove `from __future__ import annotations`

* Update Changelog

* Increment patch version number (v2.12.1)

* Revert "Increment patch version number (v2.12.1)"

This reverts commit 0999c27.

* Revert "Update Changelog"

This reverts commit 166ba04.

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>
rwgk added a commit that referenced this issue Sep 13, 2024
… independent Python/C++ bindings systems. (#5370)

* Enable type-safe interoperability between different independent Python/C++ bindings systems. (#5296)

* `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match.

* Include cleanup (mainly to resolve PyPy build failures).

* Fix clang-tidy errors.

* Resolve `error: extra

* factor out platform_abi_id.h from internals.h (no functional changes)

* factor out internals_version.h from internals.h (no functional changes)

* Update CMakeLists.txt, tests/extra_python_package/test_files.py

* Revert "factor out internals_version.h from internals.h (no functional changes)"

This reverts commit 3ccea8c.

* Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py

* `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name`

* Add PremiumTraveler

* Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h

* Expand tests: `PremiumTraveler`, `get_points()`

* Shuffle order of tests (no real changes).

* Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function.

* Use `type_caster_generic::load(self)` instead of `cast<T *>(self)`

* Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`).

* Make platform_abi_id.h completely stand-alone.

* rename exo_planet.cpp -> exo_planet_pybind11.cpp

* Add exo_planet_c_api.cpp (incomplete).

* Fix silly oversight (wrong filename in `#include`).

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); }
      | ~~~~~~           ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   14 | static PyMethodDef ThisMethodDef[]
      | ~~~~~~             ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   17 | static struct PyModuleDef ThisModuleDef = {
      | ~~~~~~                    ^
```

* Implement exo_planet_c_api GetLuggage(), GetPoints()

* Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()`

* Fix oversight.

* Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures.

* Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later.

* Small cleanup.

* Restore and add to `test_call_cpp_transporter_*()`

* Ensure #3788 does not bite again.

* `class_dunder_cpp_transporter()`: replace `obj.cast<std::string>()` with `std::string(obj)`

* Add (simple) copyright notices in all newly added files.

* Globally replace cpp_transporter with cpp_conduit

* style: pre-commit fixes

* IWYU fixes

* Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()`

* Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`.

This avoids the unicode decode/encode roundtrips:

* More robust (no decode/encode errors).

* Minor runtime optimization.

* Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes).

* Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change).

This provides an extra layer of protection against C++ ABI mismatches:

* The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions.

* The second layer is that the `typeid(std::type_info).name()`s must match between extensions.

* Fix sort order accident in tests/CMakeLists.txt

* Apply suggestions from code review

Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

* style: pre-commit fixes

* refactor: rename to _pybind_conduit_v1_

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

* Add test_home_planet_wrap_very_lonely_traveler(), test_exo_planet_pybind11_wrap_very_lonely_traveler()

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:39:32: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   10 |     py::class_<LonelyTraveler>(m, "LonelyTraveler");
      |                                ^
      |                                std::move( )
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:43:52: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   43 |     py::class_<VeryLonelyTraveler, LonelyTraveler>(m, "VeryLonelyTraveler");
      |                                                    ^
      |                                                    std::move( )
```

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

* Remove `from __future__ import annotations`

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>
henryiii added a commit that referenced this issue Sep 13, 2024
…n/C++ bindings systems. (#5296)

* `self.__cpp_transporter__()` proof of concept: Enable passing C++ pointers across extensions even if the `PYBIND11_INTERNALS_VERSION`s do not match.

* Include cleanup (mainly to resolve PyPy build failures).

* Fix clang-tidy errors.

* Resolve `error: extra

* factor out platform_abi_id.h from internals.h (no functional changes)

* factor out internals_version.h from internals.h (no functional changes)

* Update CMakeLists.txt, tests/extra_python_package/test_files.py

* Revert "factor out internals_version.h from internals.h (no functional changes)"

This reverts commit 3ccea8c.

* Remove internals_version.h from CMakeLists.txt, tests/extra_python_package/test_files.py

* `.__cpp_transporter__()` implementation: compare `pybind11_platform_abi_id`, `cpp_typeid_name`

* Add PremiumTraveler

* Rename test_cpp_transporter_traveler_type.h -> test_cpp_transporter_traveler_types.h

* Expand tests: `PremiumTraveler`, `get_points()`

* Shuffle order of tests (no real changes).

* Move `__cpp_transporter__` lambda to `py::cpp_transporter()` regular function.

* Use `type_caster_generic::load(self)` instead of `cast<T *>(self)`

* Pass `const std::type_info *` via `py::capsule` (instead of `cpp_typeid_name`).

* Make platform_abi_id.h completely stand-alone.

* rename exo_planet.cpp -> exo_planet_pybind11.cpp

* Add exo_planet_c_api.cpp (incomplete).

* Fix silly oversight (wrong filename in `#include`).

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:10:18: error: 'wrapGetLuggage' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   10 | static PyObject *wrapGetLuggage(PyObject *, PyObject *) { return PyUnicode_FromString("TODO"); }
      | ~~~~~~           ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:14:20: error: 'ThisMethodDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   14 | static PyMethodDef ThisMethodDef[]
      | ~~~~~~             ^
/__w/pybind11/pybind11/tests/exo_planet_c_api.cpp:17:27: error: 'ThisModuleDef' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace,-warnings-as-errors]
   17 | static struct PyModuleDef ThisModuleDef = {
      | ~~~~~~                    ^
```

* Implement exo_planet_c_api GetLuggage(), GetPoints()

* Move new code from test_cpp_transporter_traveler_bindings.h to pybind11/detail/type_caster_base.h, under the name `class_dunder_cpp_transporter()`

* Fix oversight.

* Unconditionally add `__cpp_transporter__` method to all `py::class_` objects, but do not include that magic method in docstring signatures.

* Back out pybind11/detail/platform_abi_id.h for now. Maximizing reusability can be handled separately, later.

* Small cleanup.

* Restore and add to `test_call_cpp_transporter_*()`

* Ensure #3788 does not bite again.

* `class_dunder_cpp_transporter()`: replace `obj.cast<std::string>()` with `std::string(obj)`

* Add (simple) copyright notices in all newly added files.

* Globally replace cpp_transporter with cpp_conduit

* style: pre-commit fixes

* IWYU fixes

* Rename `class_dunder_cpp_conduit()` -> `cpp_conduit_method()`

* Change `pybind11_platform_abi_id`, `pointer_kind` argument types from `str` to `bytes`.

This avoids the unicode decode/encode roundtrips:

* More robust (no decode/encode errors).

* Minor runtime optimization.

* Systematically rename `cap_cpp_type_info` -> `cpp_type_info_capsule` (no functional changes).

* Systematically replace `cpp_type_info_capsule` `name`: `"const std::type_info *"` -> `typeid(std::type_info).name()` (this IS a functional change).

This provides an extra layer of protection against C++ ABI mismatches:

* The first and most important layer is that the `PYBIND11_PLATFORM_ABI_ID`s must match between extensions.

* The second layer is that the `typeid(std::type_info).name()`s must match between extensions.

* Fix sort order accident in tests/CMakeLists.txt

* Apply suggestions from code review

Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

* style: pre-commit fixes

* refactor: rename to _pybind_conduit_v1_

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

* Add test_home_planet_wrap_very_lonely_traveler(), test_exo_planet_pybind11_wrap_very_lonely_traveler()

* Resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:39:32: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   10 |     py::class_<LonelyTraveler>(m, "LonelyTraveler");
      |                                ^
      |                                std::move( )
/__w/pybind11/pybind11/tests/test_cpp_conduit_traveler_bindings.h:43:52: error: parameter 'm' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param,-warnings-as-errors]
   43 |     py::class_<VeryLonelyTraveler, LonelyTraveler>(m, "VeryLonelyTraveler");
      |                                                    ^
      |                                                    std::move( )
```

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage New bug, unverified
Projects
None yet
Development

No branches or pull requests

3 participants