pybind11
pybind11 copied to clipboard
Smart Pointers for non custom types
Added a way for non custom types to take advantage of smart pointers which hopefully solves #787.
Some notes:
I know this change should update documentation and I am willing to do so, but would love to get feedback on implementation before updating docs.
For backwards compatibility with individuals using holder_helper I could create other new structs as helpers here, but it felt strange as I tried to design the interface. This was especially true due to the const vs non const get that was implemented.
I did consider making the different versions of copyable_holder_caster using SFINAE, but I couldn't seem to get it work properly. If someone thinks that would be a better approach I can attempt to do it.
I didn't add as many tests for situations that are a bit odd and will add some tests for them again if this approach is useful. One example would be doing something like the following which would create the possibility that both methods are created I think -- I am not sure how SFINAE would work here.
py::bind_vector<std::vector<Object>, std::shared_ptr<std::vector<Object>>>(m, "ObjectVec");
I just realized my implementation has some issues with the correct return_value_policy and handling correctly the lifetime of objects. I am going to attempt to correct this, so still a WIP.
I think this fixed the return value policy but I think some review here if the return_value_policy_override is going to be unexpected as I think it might be in some situations. For example:
If you have a custom binding for a class MyObject, but if it does not use a shared_ptr for internal storage, a return value of std::shared_ptr<MyObject> would have its value moved out (I think). This might be unexpected for some users.
@wjakob This is ready for a full review, but if this is is this something you don't see the library implementing let me know and I can close the PR and approach this in my work in some other way. Thanks!
@flippmoke Did you find a workaround for this? I'm surprised this PR didn't get deserved attention.
Using shared_ptr<std::vector
Oh, interesting, I never noticed this PR before. I need to look closer.
But in the meantime, for years, I worked on this, and it's used by Google since spring 2021:
https://github.com/pybind/pybind11/tree/smart_holder
Just use that. It has everything that the master branch has. All you need to do is git clone from the smart_holder branch and then use py::classh instead of py::class_ (and only when it actually matters for your use cases).
For the past couple months I've been working on getting it ready for merging into master. At the moment I'm reviewing the handling of const unique_ptr<T> &, I didn't give that enough attention before; but that's only a pretty obscure and small part of the unique_ptr/shared_ptr functionality.
@rwgk Do you have plans to merge it in master?
@rwgk Do you have plans to merge it in master?
#5339, created yesterday. Waiting for feedback.
Oh! — I only now got to look at the "Files changed" here, and only now realized what this PR is doing. It's orthogonal to the smart_holder change. I think this PR is really useful!
P.S. The title of this PR is unfortunate: what is "custom"? I misunderstood it at first glance.
I recommend working on the PR description, to concisely and carefully state what this PR is about, with a simple example.
Not sure what to write in the title exactly, but I'd definitely try to avoid a vague term like "custom".
Usually I write the PR description first, then extract a title from that.