Pybind11: [FEAT] Rework casting

[rhaschke]

Recently @rwgk revived the discussion on supporting movable custom types (and holders), see #2583, #2040, #1132.
I want to summarize the discussion so far and structure the problem into several subproblems, in the hope that we can focus our future discussion on the corresponding subproblems.

1. Fix segfaults due to mismatching holder types, i.e. fixing #1138 and #1215.
   pybind11 assumes that holder types are used consistently, i.e. once registered, a custom type needs to stick with its holder type, let it be the default unique_ptr or any custom smart ptr. However, this basic assumption is not validated (yet) and thus can cause severe segfaults if users don't obey this rule.
   I think #2644 addresses this issue in a clean and efficient manner (building on the great work #1161 from @jagerman) by validating holder compatibility at function/class definition time (runtime).

2. Support moving (rvalue-reference arguments) for custom types, allowing to (easily) wrap functions like this:

   void consume(CustomType&& object) { CustomType sink(std::move(object)); }
   

   This doesn't work out of the box (yet), but is in principle possible already right now, employing a small wrapper function as pointed out by @YannickJadoul in https://github.com/pybind/pybind11/pull/2047#issuecomment-705229376.
   My PR #2047 is an initial attempt to enable this feature. However, I recently detected an open issue with this approach.
   EDIT: In a rework, I hopefully fixed the move/copy semantics. See here for details.

3. Support moving of holder types, i.e. ownership transfer from python to C++, allowing to wrap functions like this:

   void consume(unique_ptr<CustomType>&& object) { CustomType sink(std::move(*holder.release()); }
   

   There are two possible implementations for this: #1146 (rather huge) and #2046. However, to quote @wjakob:

   Transferring ownership from Python to C++ is a super-dangerous and rather unnatural (non-pythonic) operation and intentionally not supported in pybind11.

   If implementing this, we need to ensure that all python object references of the moved holder are validated before loading. For an open issues on this, see https://github.com/pybind/pybind11/pull/2046#issuecomment-570081915.

4. A feature related to both 1. and 3. is the (implicit) conversion between different holder types as requested e.g. in https://github.com/pybind/pybind11/pull/1161#issuecomment-411138039 and proposed in https://github.com/pybind/pybind11/pull/1161#issuecomment-340309134: Instead of complaining about incompatible holder types, pybind11 should "just" auto-magically convert between them - if possible. For example, std::shared_ptr can be implicitly move-constructed from a std::unique_ptr, which indeed would make sense for function return values, i.e. auto-converting a std::unique_ptr return value into a std::shared_ptr holder.
   But, in my opinion, this is the only valid use case. If 3. is in place, argument conversion can be easily handled _explicitly_ with corresponding wrapper functions.

   Instead of a silently auto-converting between holders, maybe a new return_value_policy could be used that specifies the target holder type and then - at compile time - just adds another wrapping layer to convert the return value?

5. As pointed out by @YannickJadoul in https://github.com/pybind/pybind11/pull/2047#issuecomment-712325915, there are some more open issues with casters, e.g. #2245/#2303/#2527, or #2336.

6. EDIT(eric): pybind11 inheritance slicing; see #1333 for an example. Including it here due to current coupling to holder setup.

[YannickJadoul]

Yes, we're aware. Hopefully, this could be some project for pybind11 3.0.

Thanks for the summary, though!

[YannickJadoul]

To clarify: fixing bugs and adding minor features seems definitely possible before, ofc. But a full redesign/rework will take some time be breaking changes.

[YannickJadoul]

I've created a new "holders" label, for everything related to this type of thing. This should make it easier to find back issues and PRs (as you can search for everything with this label: https://github.com/pybind/pybind11/labels/holders).

Please apply it (or ping me or send me a message or ...), if you find anything related!

EDIT: The distinction with the already existing "casters" label isn't always obvious. Let's be careful, but in case of doubt, better have the two labels than none?

[rwgk]

Boost.Python observations (also relevant to the issues reported under #2672):

• Boost.Python has this function to convert void* pointers for polymorphic types:
  https://github.com/boostorg/python/blob/5e4f6278e01f018c32cebc4ffc3b75a743fb1042/src/object/inheritance.cpp#L390
  This functionality is completely missing from pybind11: the reinterpret and explicit casts pointed out under https://github.com/pybind/pybind11/pull/2672#issuecomment-748392993 and https://github.com/pybind/pybind11/pull/2672#issuecomment-749764131 implicitly assume that the offset as computed in the Boost.Python code is always 0.

• When passing to shared_ptr arguments, Boost.Python always constructs a new shared_ptr, using a properly cast raw pointer, and a custom deleter keeping the PyObject owning the raw pointer alive:
  https://github.com/boostorg/python/blob/5e4f6278e01f018c32cebc4ffc3b75a743fb1042/include/boost/python/converter/shared_ptr_from_python.hpp#L52
  Note that this code uses the shared_ptr aliasing constructor.
  In this way shared_ptr upcasts and downcasts work correctly, although the argument shared_ptr::use_count results tend to surprise users. Note that this mechanism is independent of the HeldType, in other words, no matter what the HeldType, it is possible to pass to shared_ptr arguments.

• With auto_ptr<T> as HeldType, Boost.Python does support passing to a auto_ptr<T> argument: the PyObject is properly disowned, although there is no dedicated error message for this situation, when methods of the disowned object are called: https://github.com/rwgk/rwgk_tbx/blob/01fd6692dbc11eb121a9aa93595b10e36f31a86f/tst_auto_ptr_holder.py#L46

• With auto_ptr as HeldType, Boost.Python does NOT support passing, e.g., a held auto_ptr<derived> to a auto_ptr<base> argument: https://github.com/rwgk/rwgk_tbx/blob/01fd6692dbc11eb121a9aa93595b10e36f31a86f/tst_auto_ptr_holder.py#L98
  When inserting prints in convert_type (object/inheritance.cpp link above), src_t shows as drvd as expected, but dst_t shows as auto_ptr<base>. Apparently the functionality to extract the raw pointer, cast it, and put it back into a new auto_ptr object, is missing from Boost.Python.

[EricCousineau-TRI]

One question: Would we consider slicing to be under this umbrella?
If so, I'd like to add Item 5, to address slicing (#1333).

As we discussed in VC, in an ideal world, this would be decoupled. However, the mechanisms for handling single and multiple inheritance are coupled due to how casting works in conjunction with object lifetime.

EDIT (@YannickJadoul): another issue about "slicing" or however we're going to call this, to be mentioned in this item: #1941

[rwgk]

One question: Would we consider slicing to be under this umbrella?

My take: it's a very important use case. It will be great if we can capture this in the redesign, too. Let's try how far we get, and drop it only if we cannot find a reasonable way to handle it?

"Slicing" is a very overloaded term. It could help the discussion if we had a more descriptive / less ambiguous term, especially to attract contributions or opinions from outsiders.

[EricCousineau-TRI]

Aye, gotcha, I assume in the context of slice() type and the syntax to support it. For now, I'll just call it "pybind11 inheritance slicing", and will ponder some more succinct terms.
FWIW, here's the Wikipedia article on general object slicing (not exactly what happens here, but akin to it): https://en.wikipedia.org/wiki/Object_slicing