-
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
[FEAT] Make enable_shared_from_this logic in custom holder types configurable #2957
Comments
Also related #2067 |
\cc @rwgk if this is something you have encountered during |
NB: The example above is subtly wrong because our c10::intrusive_ptr initializes the intrusive refcounts to ZERO not ONE, so we actually have to bump the refcount here, but other implementations (including unique_ptr) might not have done it this way. Buyer beware! |
uhm ... yes, I looked at the existing code to learn what I need to do in the smart_holder code, but it didn't click TBH. I just left it out waiting to come across something that forces me to handle Regarding custom smart-pointers in general, quoting from the current README_smart_holder.rst:
|
FYI: I'm working on #3023 to make Maybe when I'm done we could review how something like |
Cool. Intrusive_ptr isn't very complicated so I suspect most reasonable designs will work |
When defining a holder type that can be initialized from a raw pointer
T*
(https://pybind11.readthedocs.io/en/stable/advanced/smart_ptrs.html), pybind11 conventionally assumes that there is a constructorHolder(T*)
which will let it steal a raw pointer reference to initialize into the older. However, sometimes, you want to define a holder type where the raw pointer constructor doesn't have the correct semantics (e.g.,std::shared_ptr
) or you want to omit the raw pointer constructor and explicitly require users to say if they want to steal or borrow from the passed in raw pointer.Unfortunately, there is no way to currently do this; you must define another Wrapper class to work around this problem. Here is an example from our codebase, where
c10::intrusive_ptr
is a holder type but it doesn't support construction from raw pointer (instead it's a static method called reclaim) and so I have to write this boilerplate:It would be nice if there was some way to configure similar logic to enable_shared_from_this for our own type so we can bypass the raw pointer constructor entirely.
The text was updated successfully, but these errors were encountered: