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]

This uses pybind#1146 to enable the initial PRs for the systems framework Python bindings.
In the future, we may only need a subset (or none of) this PR, and may be able to revert back to upstream pybind11.

Current state of the unittests:

• The unittests pass on CPython 2 and 3 for Linux and Mac (one test still pending for CPython 3 on Mac).
• The unittests fail on PyPy and Windows. Given that Drake is not presently supported on these platforms, I'd like to defer fixing this issues until later.

---

This change is 

[EricCousineau-TRI]

+@m-chaturvedi for dev-folder feature review on testing, please.

I've paired the review down to the following files:

tests/test_class.py
tests/test_methods_and_attributes.py
tests/test_ownership_transfer.cpp
tests/test_ownership_transfer.py
tests/test_smart_ptr.cpp
tests/test_smart_ptr.py

The tests should now pass on all of TravisCI (I disabled the tests on PyPy for the time being).

---

Review status: 0 of 16 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[m-chaturvedi]

docs/advanced/classes.rst, line 1010 at r2 (raw file):

note
not?

---

Comments from Reviewable

[m-chaturvedi]

tests/test_smart_ptr.cpp, line 43 at r2 (raw file):

T *get() const {

T* ?

---

Comments from Reviewable

[m-chaturvedi]

What would be the expected behavior of something like a smart pointer to a numpy matrix of objects ? Would we need tests for that ?

---

Comments from Reviewable

[m-chaturvedi]

Reviewed 12 of 16 files at r1, 5 of 5 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[EricCousineau-TRI]

I tried out placing NumPy objects in smart pointers, and pybind11 does not support that (as it only supports custom types being wrapped in a holder shared_ptr, but Eigen types are considered "built-in"):
EricCousineau-TRI@ee6a5e1
^ This creates a compiler error.

For MI, the existing unittests pass; for unique_ptrs, it will throw the appropriate exception stating that the class will be sliced. However, for shared_ptrs, it is not working.

Would you be OK with a TODO on resolving this? (At present, we are not using any MI within Drake, at least that I'm aware of for the public-facing API.)

---

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[rdeits]

Wow, this is really cool!

I'm still concerned about the idea of trying to limit Python objects to a single reference when passing them to a function expecting a unique_ptr. Just as an example, any python object which is ever displayed in IPython (or in a Jupyter notebook) will have a reference stored in the special Out dictionary. Thus, for example, if foo(x) expects x to a unique_ptr, then the following will work in IPython/Jupyter:

> x = NewX()
> foo(x)

but the following will result in x having at least two references:

> x = NewX()
> x  # look at x
> foo(x)

This would seem to literally create Heisenbugs, since a wrapped object can be constructed and passed, but only if it is never examined. More confusingly, since the default python REPL does not store outputs, the above will work there.

I've still never encountered any Python code that required an object to have a single reference (and googling for things like "python unique reference", I can't find any code either). There are just so many ways for additional references to crop up that getting the refcount to exactly 1 seems impractical.

Or am I missing something?

[EricCousineau-TRI]

Or am I missing something?

Nope, not at all, you're spot on!

My desire was to have the Python bindings play well with the existing Systems framework, but then transition it over to use shared_ptr to better support REPL workflows. There is a way to get this all to work using pybind's keep_alive mechanism whenever you add an object to a container; however, that means you'll be dragging that container around for the entire lifetime of the containee, which may yield some memory issues.
Ultimately, I wanted to have flexibility and start with the more conservative set of features (lifetime-limitations, as captured here in the Python Systems PR), and then expand later (and not require everything to automatically use shared_ptr, since there's no way to get a pointer out of a shared_ptr once you put it in there).

---

Review status: 12 of 18 files reviewed at latest revision, 2 unresolved discussions.

---

docs/advanced/classes.rst, line 1010 at r2 (raw file):

Previously, m-chaturvedi (Mmanu Chaturvedi) wrote…

note
not?

Done.

---

tests/test_smart_ptr.cpp, line 43 at r2 (raw file):

Previously, m-chaturvedi (Mmanu Chaturvedi) wrote…

T *get() const {

T* ?

Done.

---

Comments from Reviewable

[EricCousineau-TRI]

@m-chaturvedi Seems that the behavior I was trying to test in my branch is also broken in master. I've documented this with a TODO, and will file a bug (with a PR to capture this at some point).

---

Review status: 12 of 18 files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[rdeits]

Ok, well, I'm convinced that you've given this more thought than I have, so I'll trust your judgment.

[m-chaturvedi]

Reviewed 6 of 7 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[rdeits]

to unblock merging

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[jadecastro]

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[m-chaturvedi]

Done via Reviewable.