[FEAT] Rework casting
Nobody has claimed this yet.
- 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.
-
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 defaultunique_ptror 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). -
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. -
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. -
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_ptrcan be implicitly move-constructed from astd::unique_ptr, which indeed would make sense for function return values, i.e. auto-converting astd::unique_ptrreturn value into astd::shared_ptrholder.
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_policycould be used that specifies the target holder type and then - at compile time - just adds another wrapping layer to convert the return value? -
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.
-
EDIT(eric): pybind11 inheritance slicing; see #1333 for an example. Including it here due to current coupling to holder setup.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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