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

py::smart_holder std::shared_ptr deleter simplification & optimization. #3041

Merged
merged 1 commit into from
Jun 17, 2021

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Jun 17, 2021

Replacing guarded_builtin_delete and guarded_custom_deleter with a simpler guarded_delete.

The primary motivation for this change was to enable straightforward use of std::get_deleter, but this happened to lead to a general simplification & optimization.

guarded_delete only manages a simple function pointer and a bool. shared_ptr<bool> is no longer needed and removed from py::smart_holder.

This change was developed and copied from PR #3023. It sets the stage for changing py::smart_holder to play nicely with enable_shared_from_this.

For completeness, calling out two collateral tweaks that are not strictly needed for the deleter change:

-    smart_holder &operator=(smart_holder &&) = default;
+    smart_holder &operator=(smart_holder &&) = delete;
     static smart_holder from_raw_ptr_unowned(T *raw_ptr) {                     
-        hld.vptr.reset(raw_ptr, guarded_builtin_delete<T>(hld.vptr_deleter_armed_flag_ptr));
+        hld.vptr.reset(raw_ptr, [](void *) {});                                

@rwgk
Copy link
Collaborator Author

rwgk commented Jun 17, 2021

This change will get reviewed Google-internally.

@rwgk rwgk merged commit 4f61912 into pybind:smart_holder Jun 17, 2021
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Jun 17, 2021
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Jun 17, 2021
@rwgk rwgk deleted the sh_guarded_deleter_simplification branch June 17, 2021 00:57
rwgk added a commit to rwgk/pybind11 that referenced this pull request Jun 20, 2021
rwgk added a commit to rwgk/pybind11 that referenced this pull request Jun 21, 2021
rwgk added a commit to rwgk/pybind11 that referenced this pull request Jun 23, 2021
rwgk added a commit to rwgk/pybind11 that referenced this pull request Jun 24, 2021
rwgk added a commit to rwgk/pybind11 that referenced this pull request Jun 28, 2021
rwgk added a commit to rwgk/pybind11 that referenced this pull request Jun 29, 2021
rwgk added a commit to rwgk/pybind11 that referenced this pull request Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant