WIP: Support ownership transfer between C++ and Python with shared_ptr<T> and unique_ptr<T> for pure C++ instances and single-inheritance instances

[EricCousineau-TRI]

UPDATE (2020-12-28): Decreed as stale.
For a more up-to-date and working implementation, see https://github.com/RobotLocomotion/pybind11/blob/drake/README_DRAKE.md (permalink)

---

This addresses #1132 and #1145 (and accommodates #1138 / incorporates #1139).

All unittests pass on my system (Python 2.7, release + debug).

Todo

• Minimize changes to internals: detail::type_info and detail::instance
• Implement test cases to examine ownership behavior with and without pybind11::wrapper<Base>
• Squash commits + rebase on master, make PR to master.

[wjakob]

I don't think it's going to be feasible for us to even consider a commit of this scale to support moving std::unique_ptr (which should be discussed too -- I'm not really sure that this feature makes sense to me).

[EricCousineau-TRI]

I understand on the commit scale. I can certainly break this into smaller components to take this out of a WIP stage. (I can also collapse the commits - those were me just prototyping small tests as I was learning some of the internals of pybind11.)

For moving unique_ptr<>, can I ask what part does not make sense? (Well, aside from the bloat of the implementation, and the oddity of signaling a move via a list is a tad kludgy...)
This permits the equivalent of #971, but a with a few more checks, and supporting single inheritance.

This also includes preventing Python-subclass aliasing that would arise from passing shared_ptr<>s from Python to C++, which at present would fail silently. (This is my main focus at present, but I would still like to enable casting unique_ptr<>-held instances to permit full ownership transfer.) The solution shares an implementation with the unique_ptr<> approach.
Additionally, it addresses #1138 more robustly, and provides some limited mixing of compatible holder types (unique_ptr<T> -> shared_ptr<T>).

[EricCousineau-TRI]

I've squashed the commits in this branch. All of the granular experimental history is in feature/py_move_shared-wip on my repo.

The workflow for derived-type aliasing, specifically with shared_ptr<>, is now present in a more simplified unittest (hacky with ConstructorStats workarounds, but works):
https://github.com/pybind/pybind11/pull/1146/files#diff-5dd500b0d477a5b264d862e0157b4847R31

Still on my TODO is the unique_ptr<> modifications captured in pybind11-style unittest; however, I'll defer that pending more discussion on if you think it doesn't belong. (I'd like to have it, but I understand that at present it's a tad messy.)

[wjakob]

I don't think squashing everything into a single commit addresses the problem, which is really the sheer size of the diff. I'll try to take a look in the next 1-2 weeks regarding the functional aspects (sorry for the delay, I have an unrelated deadline ATM).

[EricCousineau-TRI]

Sorry, I didn't mean to imply that squashing was the only problem :)
I will see if I can provide a few more concise unittests to help bubble up the features that I'm focused on; if possible, I will see if I can stage these features, possibly layering the commits in this PR.

Thanks!

[jagerman]

One thing that I'm missing is a concise overview of what this feature is all about. Can I suggest that you write up a summary of a few paragraphs aimed at someone who has some pybind experience, but not a core developer that describes the motivation, feature, and simple implementation. (The reason I ask for a experienced-but-not-core-developer target is that this isn't just for evaluation of the PR: it would also form the basis of the docs/* content that would eventually need to be added before the PR could be merged).

[EricCousineau-TRI]

Most certainly!
I will type something up similar to what I had in #1132, but include the case for shared_ptr<T>.
For development, I will also try to identify the separate features, and use that as a basis for a PR-train if this is deemed appropriate.

Thank you for your patience and for considering this!

[EricCousineau-TRI]

I've added a quick draft of the main features of what this PR could introduce at a user-facing level. If you have time, please let me know if you have any comments!
https://github.com/EricCousineau-TRI/pybind11/blob/feature/py_move_shared-wip/docs/advanced/classes.rst#virtual-inheritance-and-lifetime

[EricCousineau-TRI]

I have updated the documentation, and included details on the holder stuff for ownership transfer as well:
https://github.com/EricCousineau-TRI/pybind11/blob/feature/py_move_shared-wip/docs/advanced/smart_ptrs.rst

EDIT: If you would like to see the overview in a different form, please let me know!

[EricCousineau-TRI]

@ppt-adsk mentioned that they have ran into a similar issue, at least for inheritance with shared_ptrs.

That being said, can I ask if there is any way that I could carve up the overview docs or this PR to make it easier to review?
I can re-vamp the PR to focus solely on shared_ptr<>, and defer mixing holder types to the convergence of #1161.

[EricCousineau-TRI]

I'm going to decree this PR as stale. I've updated the overview to indicate where to look for more up-to-date discussions.