Revising the design of type_caster<T>::load()
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 18k
- Forks
- 2.3k
- Avg merge
- 5d 17h
- Merged PRs (30d)
- 10
Description
Currently, type casters defined using PYBIND11_TYPE_CASTER require that the C++ type is default-constructible and copy/move-assignable. Apart from the fact that not all types fulfill these requirements, doing default init + assign is less efficient than directly constructing an object (both binary size and runtime overhead). There are also possible complications as in the case of #850 where an additional helper struct and specialization are required but could be avoided with a different load() design.
The proposal
The idea is to make type_caster<T>::load() a static function which returns an optional-like container. Looking only at the load-specific parts, a type caster currently looks like this:
struct caster {
T value;
operator type*();
operator type&();
template <typename U> using cast_op_type;
bool load(handle src, bool convert);
}
This proposal changes it to:
struct caster {
static maybe<T> load(handle src, bool convert);
}
Where maybe<T, ExtractionPolicy> is an optional-like container with some simplifications and specializations for the purpose of pybind11's casters (named maybe to avoid confusion with the standard optional). Here, the ExtractionPolicy template parameter takes on the job of cast_op_type and the cast operators. The default policy is the same as defined by PYBIND11_TYPE_CASTER which makes maybe<T> the only thing needed for user-defined casters. This also drastically reduces the footprint of the PYBIND11_TYPE_CASTER macro.
This would also simplify usage:
// currently
U value;
bool load(handle src, bool convert) {
make_caster<T> subcaster;
if (subcaster.load(src, convert)) {
value = cast_op<T>(subcaster);
return true;
}
return false;
}
// proposed
static maybe<U> load(handle src, bool convert) {
if (auto maybe_value = make_caster<T>::load(src, convert))
return maybe_value.extract<T>();
return {};
}
The behavior of maybe::extract<T>() is determined by the ExtractionPolicy. Given that these containers have a very special purpose and are only used once, the default policy could move when T is a value or rvalue ref (to simplify caster code).
Backward compatibility
Existing user-defined casters would still function either by inserting a compatibility layer into PYBIND11_TYPE_CASTER or by using SFINAE in pybind11 internals (only if compatibility is required for user-defined casters which do not use the PYBIND11_TYPE_CASTER macro).
If this whole proposal sounds good, this might also be a good time to consider moving user-defined casters out of the detail namespace.
PR
I have a rough working prototype and I can turn this into a PR if the proposal sounds acceptable. I think that reducing the footprint of the PYBIND11_TYPE_CASTER macro and lifting the default constructor + assignment requirements would be a nice improvement.
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 reviewing the current type_caster::load() design and the PYBIND11_TYPE_CASTER macro described in the issue. Compare the proposed maybe return model with the existing caster behavior, including compatibility for user-defined casters. Done means the redesign removes the default-construction and assignment requirements while preserving existing caster compatibility.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100