[FEAT] Rework casting · Issue #2646 · pybind/pybind11 (original) (raw)

Skip to content

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sign up

Appearance settings

@rhaschke

Description

@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 pybind11 wrapped method segfaults if a function returns unique_ptr when holder is shared_ptr #1138 and C++ object is destructed before it can be used, when returned as a shared_ptr and using default holder type #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 Detect and fail if using mismatched holders #2644 addresses this issue in a clean and efficient manner (building on the great work Detect and fail if using mismatched holders #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 [WIP] Towards a solution for custom-type rvalue arguments #2047 (comment).
    My PR [WIP] Towards a solution for custom-type rvalue arguments #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&& object) { CustomType sink(std::move(*holder.release()); }
    There are two possible implementations for this: WIP: Support ownership transfer between C++ and Python with shared_ptr and unique_ptr for pure C++ instances and single-inheritance instances #1146 (rather huge) and Towards a solution for rvalue holder arguments in wrapped functions #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 Towards a solution for rvalue holder arguments in wrapped functions #2046 (comment).

  4. A feature related to both 1. and 3. is the (implicit) conversion between different holder types as requested e.g. in Detect and fail if using mismatched holders #1161 (comment) and proposed in Detect and fail if using mismatched holders #1161 (comment): 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 [WIP] Towards a solution for custom-type rvalue arguments #2047 (comment), there are some more open issues with casters, e.g. passing containers of char * (e.g. std::vector<char *>) does not work #2245/Fix casters of STL containers with char* #2303/[BUG] Keep pybind11 type casters alive recursively when needed #2527, or Casting from an unregistered type does not throw. #2336.
  6. EDIT(eric): pybind11 inheritance slicing; see [BUG] Problem when creating derived Python objects from C++ (inheritance slicing) #1333 for an example. Including it here due to current coupling to holder setup.