-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[DEV-master-smart_holder] Bake smart_holder functionality into class_
and type_caster_base
#5213
Conversation
…mart_holder (test fails)
…stance() specialization. Also bring in dynamic_raw_ptr_cast_if_possible.h. Does not build because try_initialization_using_shared_from_this() is still missing.
…holder_type_casters.h. With this test_wip builds and succeeds.
skip-checks: true
Current state testing with the Google-internal toolchain, using ASAN: 7 failing tests:
All other tests run ASAN-clean. For completeness, this is the list of passing tests: 40 passing tests:
|
…ch (does not build).
…with is_same<Holder<Class>, smart_holder> (still does not build).
…() from smart_holder_type_casters.h (with this test_wip builds and runs successfully).
…l (4) BAKEIN_BREAK from test_factory_constructors.py (test_factory_constructors builds and runs successfully).
Tracking progress: test_factory_constructors builds and runs successfully WITH ALL BAKEIN_BREAK removed. This is still the same: |
…ly_holder_caster<unique_ptr>. No functional changes.
…_ptr_to_python() there, add smart_holder_from_unique_ptr()
…ipped tests pass locally with C++17).
…(replaces BAKEIN_BREAK).
… in most places.
… the `std::shared_ptr` specialization and remove `pytest.skip("BAKEIN_EXPECTED: ...)` in test_smart_ptr.py
…ALS_WITH_SMART_HOLDER_SUPPORT is not defined.
…S_WITH_SMART_HOLDER_SUPPORT is not defined.
…ORT is not defined.
…DULE_LOCAL_ID`" `"_sh_baked_in"` is no longer needed because the smart_holder functionality is now tied to the `PYBIND11_INTERNALS_VERSION`. This reverts commit 884305e.
…asters_h` namespace.
…INTERNALS_WITH_SMART_HOLDER_SUPPORT` block.
…st_class_sh_property_bakein) Corresponding to PR pybind#5213 commit 3406be6 Reduced from use cases in the wild: * `test_readonly_char6_member()`: https://github.com/pytorch/pytorch/blob/4410c44ae6fd8eb36f2358ac76f7d988ca7537c5/torch/csrc/cuda/Module.cpp#L961 * `test_readonly_const_char_ptr_member()`: https://github.com/facebookresearch/nle/blob/862a439a84f52abca94d1f744d57061da12c5831/include/permonst.h#L43
… as in test_class_sh_property_bakein.cpp,py).
…ss_sh_property_bakein) (#5256) Corresponding to PR #5213 commit 3406be6 Reduced from use cases in the wild: * `test_readonly_char6_member()`: https://github.com/pytorch/pytorch/blob/4410c44ae6fd8eb36f2358ac76f7d988ca7537c5/torch/csrc/cuda/Module.cpp#L961 * `test_readonly_const_char_ptr_member()`: https://github.com/facebookresearch/nle/blob/862a439a84f52abca94d1f744d57061da12c5831/include/permonst.h#L43
…: 1. To avoid compiler warnings for unused code in the unnamed namespace. 2. To avoid clang-format changes.
… To avoid compiler warnings for unused code in the unnamed namespace. 2. To avoid clang-format changes. (#5258)
class_
and type_caster_base
class_
and type_caster_base
Closing, as explained in the PR description. All checks have passed: 2 skipped and 155 successful checks CI — https://github.com/pybind/pybind11/actions/runs/10038035526 CI-SH-DEF — https://github.com/pybind/pybind11/actions/runs/10038035539 |
* Factor out detail/value_and_holder.h (from detail/type_caster_base.h) This is in support of PR #5213: * trampoline_self_life_support.h depends on value_and_holder.h * type_caster_base.h depends on trampoline_self_life_support.h * Fix a minor and inconsequential inconsistency in `copyable_holder_caster`: the correct `load_value()` return type is `void` (as defined in `type_caster_generic`) For easy future reference, this is the long-standing inconsistency: * https://github.com/pybind/pybind11/blob/dbf848aff7c37ef8798bc9459a86193e28b1032f/include/pybind11/detail/type_caster_base.h#L634 * https://github.com/pybind/pybind11/blob/dbf848aff7c37ef8798bc9459a86193e28b1032f/include/pybind11/cast.h#L797 Noticed in passing while working on PR #5213. * Add `DANGER ZONE` comment in detail/init.h, similar to a comment added on the smart_holder branch (all the way back in 2021).
Description
This PR was used to rebuild the smart_holder functionality on top of the current master branch, pulling code fragments from the smart_holder branch. The approach was to pull one or more tests at a time from the smart_holder branch, then make them work by pulling production code fragments from the smart_holder branch and bake them into
class_
and the code aroundtype_caster_base
.Next step: Close this PR and continue the work under #5257.
Milestone reached at commit c4c3d9a under this PR:
Testing via GHA passes:
Google-global testing passes, with several minor Google-internal adjustments outside the pybind11 source tree. Internal testing ID: OCL:654225008:BASE:654373952:1721520078863:60337b49
pytest output when building with
-DPYBIND11_INTERNALS_VERSION=5
(i.e. without smart_holder functionality):Suggested changelog entry: