pybind / pybind/pybind11

[FEAT] Rework casting

Open
#2,646 13 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement holders policy question smart holder
Dominant language
C++
Stars
18k
Forks
2.3k
Avg merge
5d 17h
Merged PRs (30d)
10

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the linked discussions and pull requests, especially #1138, #1215, #2047, #2046, #1161, and #1333, then trace the caster and holder setup areas they identify. The issue contains six related subproblems, so first narrow it to one with an agreed scope. Done requires a defined design and focused validation for that selected casting or holder behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, python
Domain
backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.