From 8cff8c70e42fb904c6d6c6df48924118dc209713 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Mon, 13 Oct 2025 18:10:37 -0600 Subject: [PATCH 1/3] type_caster_generic: add cast_sources abstraction --- include/pybind11/cast.h | 30 ++-- include/pybind11/detail/type_caster_base.h | 176 +++++++++++++-------- 2 files changed, 120 insertions(+), 86 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index f6e4aed1b9..4708101d80 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1011,15 +1011,12 @@ struct copyable_holder_caster< static handle cast(const std::shared_ptr &src, return_value_policy policy, handle parent) { const auto *ptr = src.get(); - auto st = type_caster_base::src_and_type(ptr); - if (st.second == nullptr) { - return handle(); // no type info: error will be set already - } - if (st.second->holder_enum_v == detail::holder_enum_t::smart_holder) { + typename type_caster_base::cast_sources srcs{ptr}; + if (srcs.creates_smart_holder()) { return smart_holder_type_caster_support::smart_holder_from_shared_ptr( - src, policy, parent, st); + src, policy, parent, srcs.result); } - return type_caster_base::cast_holder(ptr, &src); + return type_caster_base::cast_holder(srcs, &src); } // This function will succeed even if the `responsible_parent` does not own the @@ -1195,21 +1192,12 @@ struct move_only_holder_caster< static handle cast(std::unique_ptr &&src, return_value_policy policy, handle parent) { auto *ptr = src.get(); - auto st = type_caster_base::src_and_type(ptr); - if (st.second == nullptr) { - return handle(); // no type info: error will be set already - } - if (st.second->holder_enum_v == detail::holder_enum_t::smart_holder) { + typename type_caster_base::cast_sources srcs{ptr}; + if (srcs.creates_smart_holder()) { return smart_holder_type_caster_support::smart_holder_from_unique_ptr( - std::move(src), policy, parent, st); - } - return type_caster_generic::cast(st.first, - return_value_policy::take_ownership, - {}, - st.second, - nullptr, - nullptr, - std::addressof(src)); + std::move(src), policy, parent, srcs.result); + } + return type_caster_base::cast_holder(srcs, &src); } static handle diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 14235ab82d..afa1ecc30a 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -920,21 +920,100 @@ class type_caster_generic { bool load(handle src, bool convert) { return load_impl(src, convert); } - PYBIND11_NOINLINE static handle cast(const void *_src, + // Information about how cast() can obtain its source object + struct cast_sources { + // A type-erased pointer and the type it points to + struct source { + const void *obj; + const std::type_info *type; + }; + + // Use the given pointer with its compile-time type, possibly downcast + // via polymorphic_type_hook() + template + cast_sources(const itype *ptr); // NOLINT(google-explicit-constructor) + + // Use the given pointer and type + // NOLINTNEXTLINE(google-explicit-constructor) + cast_sources(const source &orig) : original(orig) { result = resolve(); } + + // Use the given object and pybind11 type info. NB: if tinfo is null, + // this does not provide enough information to use a foreign type or + // to render a useful error message + cast_sources(const void *obj, const detail::type_info *tinfo) + : original{obj, tinfo ? tinfo->cpptype : nullptr}, result{obj, tinfo} {} + + // The object passed to cast(), with its static type. + // original.type must not be null if resolve() will be called. + // original.obj may be null if we're converting nullptr to a Python None + source original; + + // A more-derived version of `original` provided by a + // polymorphic_type_hook. downcast.type may be null if this is not + // a relevant concept for the current cast. + source downcast{}; + + // The source to use for this cast, and the corresponding pybind11 + // type_info. If the type_info is null, then pybind11 doesn't know + // about this type. + std::pair result; + + // Returns true if the cast will use a pybind11 type that uses + // a smart holder. + bool creates_smart_holder() const { + return result.second != nullptr + && result.second->holder_enum_v == detail::holder_enum_t::smart_holder; + } + + private: + std::pair resolve() { + if (downcast.type) { + if (same_type(*original.type, *downcast.type)) { + downcast.type = nullptr; + } else if (const auto *tpi = get_type_info(*downcast.type)) { + return {downcast.obj, tpi}; + } + } + if (const auto *tpi = get_type_info(*original.type)) { + return {original.obj, tpi}; + } + return {nullptr, nullptr}; + } + }; + + static handle cast(const void *src, + return_value_policy policy, + handle parent, + const detail::type_info *tinfo, + void *(*copy_constructor)(const void *), + void *(*move_constructor)(const void *), + const void *existing_holder = nullptr) { + cast_sources srcs{src, tinfo}; + return cast(srcs, policy, parent, copy_constructor, move_constructor, existing_holder); + } + + PYBIND11_NOINLINE static handle cast(const cast_sources &srcs, return_value_policy policy, handle parent, - const detail::type_info *tinfo, void *(*copy_constructor)(const void *), void *(*move_constructor)(const void *), const void *existing_holder = nullptr) { - if (!tinfo) { // no type info: error will be set already + if (!srcs.result.second) { + // No pybind11 type info. Raise an exception. + std::string tname = srcs.downcast.type ? srcs.downcast.type->name() + : srcs.original.type ? srcs.original.type->name() + : ""; + detail::clean_type_id(tname); + std::string msg = "Unregistered type : " + tname; + set_error(PyExc_TypeError, msg.c_str()); return handle(); } - void *src = const_cast(_src); + void *src = const_cast(srcs.result.first); if (src == nullptr) { return none().release(); } + const type_info *tinfo = srcs.result.second; if (handle registered_inst = find_registered_python_instance(src, tinfo)) { return registered_inst; @@ -1207,25 +1286,6 @@ class type_caster_generic { return false; } - // Called to do type lookup and wrap the pointer and type in a pair when a dynamic_cast - // isn't needed or can't be used. If the type is unknown, sets the error and returns a pair - // with .second = nullptr. (p.first = nullptr is not an error: it becomes None). - PYBIND11_NOINLINE static std::pair - src_and_type(const void *src, - const std::type_info &cast_type, - const std::type_info *rtti_type = nullptr) { - if (auto *tpi = get_type_info(cast_type)) { - return {src, const_cast(tpi)}; - } - - // Not found, set error: - std::string tname = rtti_type ? rtti_type->name() : cast_type.name(); - detail::clean_type_id(tname); - std::string msg = "Unregistered type : " + tname; - set_error(PyExc_TypeError, msg.c_str()); - return {nullptr, nullptr}; - } - const type_info *typeinfo = nullptr; const std::type_info *cpptype = nullptr; void *value = nullptr; @@ -1520,6 +1580,20 @@ struct polymorphic_type_hook : public polymorphic_type_hook_base {}; PYBIND11_NAMESPACE_BEGIN(detail) +template +type_caster_generic::cast_sources::cast_sources(const itype *ptr) : original{ptr, &typeid(itype)} { + // If this is a base pointer to a derived type, and the derived type is + // registered with pybind11, we want to make the full derived object + // available. In the typical case where itype is polymorphic, we get the + // correct derived pointer (which may be != base pointer) by a dynamic_cast + // to most derived type. If itype is not polymorphic, a user-provided + // specialization of polymorphic_type_hook can do the same thing. + // If there is no downcast to perform, then the default hook will leave + // derived.type set to nullptr, which causes us to ignore derived.obj. + downcast.obj = polymorphic_type_hook::get(ptr, downcast.type); + result = resolve(); +} + /// Generic type caster for objects stored on the heap template class type_caster_base : public type_caster_generic { @@ -1531,6 +1605,11 @@ class type_caster_base : public type_caster_generic { type_caster_base() : type_caster_base(typeid(type)) {} explicit type_caster_base(const std::type_info &info) : type_caster_generic(info) {} + struct cast_sources : type_caster_generic::cast_sources { + // NOLINTNEXTLINE(google-explicit-constructor) + cast_sources(const itype *ptr) : type_caster_generic::cast_sources(ptr) {} + }; + static handle cast(const itype &src, return_value_policy policy, handle parent) { if (policy == return_value_policy::automatic || policy == return_value_policy::automatic_reference) { @@ -1543,50 +1622,17 @@ class type_caster_base : public type_caster_generic { return cast(std::addressof(src), return_value_policy::move, parent); } - // Returns a (pointer, type_info) pair taking care of necessary type lookup for a - // polymorphic type (using RTTI by default, but can be overridden by specializing - // polymorphic_type_hook). If the instance isn't derived, returns the base version. - static std::pair src_and_type(const itype *src) { - const auto &cast_type = typeid(itype); - const std::type_info *instance_type = nullptr; - const void *vsrc = polymorphic_type_hook::get(src, instance_type); - if (instance_type && !same_type(cast_type, *instance_type)) { - // This is a base pointer to a derived type. If the derived type is registered - // with pybind11, we want to make the full derived object available. - // In the typical case where itype is polymorphic, we get the correct - // derived pointer (which may be != base pointer) by a dynamic_cast to - // most derived type. If itype is not polymorphic, we won't get here - // except via a user-provided specialization of polymorphic_type_hook, - // and the user has promised that no this-pointer adjustment is - // required in that case, so it's OK to use static_cast. - if (const auto *tpi = get_type_info(*instance_type)) { - return {vsrc, tpi}; - } - } - // Otherwise we have either a nullptr, an `itype` pointer, or an unknown derived pointer, - // so don't do a cast - return type_caster_generic::src_and_type(src, cast_type, instance_type); - } - - static handle cast(const itype *src, return_value_policy policy, handle parent) { - auto st = src_and_type(src); - return type_caster_generic::cast(st.first, + static handle cast(const cast_sources &srcs, return_value_policy policy, handle parent) { + return type_caster_generic::cast(srcs, policy, parent, - st.second, - make_copy_constructor(src), - make_move_constructor(src)); - } - - static handle cast_holder(const itype *src, const void *holder) { - auto st = src_and_type(src); - return type_caster_generic::cast(st.first, - return_value_policy::take_ownership, - {}, - st.second, - nullptr, - nullptr, - holder); + make_copy_constructor((const itype *) nullptr), + make_move_constructor((const itype *) nullptr)); + } + + static handle cast_holder(const cast_sources &srcs, const void *holder) { + auto policy = return_value_policy::take_ownership; + return type_caster_generic::cast(srcs, policy, {}, nullptr, nullptr, holder); } template From 5a92e40324130129f9ed78889885fe7fdcdd1bfb Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Tue, 14 Oct 2025 17:43:23 -0600 Subject: [PATCH 2/3] Respond to code review comments --- include/pybind11/detail/type_caster_base.h | 180 +++++++++++---------- 1 file changed, 95 insertions(+), 85 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index afa1ecc30a..d257f01d6f 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -540,6 +540,74 @@ PYBIND11_NOINLINE handle get_object_handle(const void *ptr, const detail::type_i }); } +// Information about how type_caster_generic::cast() can obtain its source object +struct cast_sources { + // A type-erased pointer and the type it points to + struct raw_source { + const void *cppobj; + const std::type_info *cpptype; + }; + + // A C++ pointer and the Python type info we will convert it to; + // we expect that cppobj points to something of type tinfo->cpptype + struct resolved_source { + const void *cppobj; + const type_info *tinfo; + }; + + // Use the given pointer with its compile-time type, possibly downcast + // via polymorphic_type_hook() + template + cast_sources(const itype *ptr); // NOLINT(google-explicit-constructor) + + // Use the given pointer and type + // NOLINTNEXTLINE(google-explicit-constructor) + cast_sources(const raw_source &orig) : original(orig) { result = resolve(); } + + // Use the given object and pybind11 type info. NB: if tinfo is null, + // this does not provide enough information to use a foreign type or + // to render a useful error message + cast_sources(const void *obj, const detail::type_info *tinfo) + : original{obj, tinfo ? tinfo->cpptype : nullptr}, result{obj, tinfo} {} + + // The object passed to cast(), with its static type. + // original.type must not be null if resolve() will be called. + // original.obj may be null if we're converting nullptr to a Python None + raw_source original; + + // A more-derived version of `original` provided by a + // polymorphic_type_hook. downcast.type may be null if this is not + // a relevant concept for the current cast. + raw_source downcast{}; + + // The source to use for this cast, and the corresponding pybind11 + // type_info. If the type_info is null, then pybind11 doesn't know + // about this type. + resolved_source result; + + // Returns true if the cast will use a pybind11 type that uses + // a smart holder. + bool creates_smart_holder() const { + return result.tinfo != nullptr + && result.tinfo->holder_enum_v == detail::holder_enum_t::smart_holder; + } + +private: + resolved_source resolve() { + if (downcast.cpptype) { + if (same_type(*original.cpptype, *downcast.cpptype)) { + downcast.cpptype = nullptr; + } else if (const auto *tpi = get_type_info(*downcast.cpptype)) { + return {downcast.cppobj, tpi}; + } + } + if (const auto *tpi = get_type_info(*original.cpptype)) { + return {original.cppobj, tpi}; + } + return {nullptr, nullptr}; + } +}; + // Forward declarations void keep_alive_impl(handle nurse, handle patient); inline PyObject *make_new_instance(PyTypeObject *type); @@ -602,18 +670,18 @@ template handle smart_holder_from_unique_ptr(std::unique_ptr &&src, return_value_policy policy, handle parent, - const std::pair &st) { + const cast_sources::resolved_source &cs) { if (policy == return_value_policy::copy) { throw cast_error("return_value_policy::copy is invalid for unique_ptr."); } if (!src) { return none().release(); } - // st.first is the subobject pointer appropriate for tinfo (may differ from src.get() + // cs.cppobj is the subobject pointer appropriate for tinfo (may differ from src.get() // under MI/VI). Use this for Python identity/registration, but keep ownership on T*. - void *src_raw_void_ptr = const_cast(st.first); - assert(st.second != nullptr); - const detail::type_info *tinfo = st.second; + void *src_raw_void_ptr = const_cast(cs.cppobj); + assert(cs.tinfo != nullptr); + const detail::type_info *tinfo = cs.tinfo; if (handle existing_inst = find_registered_python_instance(src_raw_void_ptr, tinfo)) { auto *self_life_support = tinfo->get_trampoline_self_life_support(src.get()); if (self_life_support != nullptr) { @@ -660,20 +728,20 @@ template handle smart_holder_from_unique_ptr(std::unique_ptr &&src, return_value_policy policy, handle parent, - const std::pair &st) { + const cast_sources::resolved_source &cs) { return smart_holder_from_unique_ptr( std::unique_ptr(const_cast(src.release()), std::move(src.get_deleter())), // Const2Mutbl policy, parent, - st); + cs); } template handle smart_holder_from_shared_ptr(const std::shared_ptr &src, return_value_policy policy, handle parent, - const std::pair &st) { + const cast_sources::resolved_source &cs) { switch (policy) { case return_value_policy::automatic: case return_value_policy::automatic_reference: @@ -692,11 +760,11 @@ handle smart_holder_from_shared_ptr(const std::shared_ptr &src, return none().release(); } - // st.first is the subobject pointer appropriate for tinfo (may differ from src.get() + // cs.cppobj is the subobject pointer appropriate for tinfo (may differ from src.get() // under MI/VI). Use this for Python identity/registration, but keep ownership on T*. - void *src_raw_void_ptr = const_cast(st.first); - assert(st.second != nullptr); - const detail::type_info *tinfo = st.second; + void *src_raw_void_ptr = const_cast(cs.cppobj); + assert(cs.tinfo != nullptr); + const detail::type_info *tinfo = cs.tinfo; if (handle existing_inst = find_registered_python_instance(src_raw_void_ptr, tinfo)) { // PYBIND11:REMINDER: MISSING: Enforcement of consistency with existing smart_holder. // PYBIND11:REMINDER: MISSING: keep_alive. @@ -723,11 +791,11 @@ template handle smart_holder_from_shared_ptr(const std::shared_ptr &src, return_value_policy policy, handle parent, - const std::pair &st) { + const cast_sources::resolved_source &cs) { return smart_holder_from_shared_ptr(std::const_pointer_cast(src), // Const2Mutbl policy, parent, - st); + cs); } struct shared_ptr_parent_life_support { @@ -920,67 +988,6 @@ class type_caster_generic { bool load(handle src, bool convert) { return load_impl(src, convert); } - // Information about how cast() can obtain its source object - struct cast_sources { - // A type-erased pointer and the type it points to - struct source { - const void *obj; - const std::type_info *type; - }; - - // Use the given pointer with its compile-time type, possibly downcast - // via polymorphic_type_hook() - template - cast_sources(const itype *ptr); // NOLINT(google-explicit-constructor) - - // Use the given pointer and type - // NOLINTNEXTLINE(google-explicit-constructor) - cast_sources(const source &orig) : original(orig) { result = resolve(); } - - // Use the given object and pybind11 type info. NB: if tinfo is null, - // this does not provide enough information to use a foreign type or - // to render a useful error message - cast_sources(const void *obj, const detail::type_info *tinfo) - : original{obj, tinfo ? tinfo->cpptype : nullptr}, result{obj, tinfo} {} - - // The object passed to cast(), with its static type. - // original.type must not be null if resolve() will be called. - // original.obj may be null if we're converting nullptr to a Python None - source original; - - // A more-derived version of `original` provided by a - // polymorphic_type_hook. downcast.type may be null if this is not - // a relevant concept for the current cast. - source downcast{}; - - // The source to use for this cast, and the corresponding pybind11 - // type_info. If the type_info is null, then pybind11 doesn't know - // about this type. - std::pair result; - - // Returns true if the cast will use a pybind11 type that uses - // a smart holder. - bool creates_smart_holder() const { - return result.second != nullptr - && result.second->holder_enum_v == detail::holder_enum_t::smart_holder; - } - - private: - std::pair resolve() { - if (downcast.type) { - if (same_type(*original.type, *downcast.type)) { - downcast.type = nullptr; - } else if (const auto *tpi = get_type_info(*downcast.type)) { - return {downcast.obj, tpi}; - } - } - if (const auto *tpi = get_type_info(*original.type)) { - return {original.obj, tpi}; - } - return {nullptr, nullptr}; - } - }; - static handle cast(const void *src, return_value_policy policy, handle parent, @@ -998,22 +1005,22 @@ class type_caster_generic { void *(*copy_constructor)(const void *), void *(*move_constructor)(const void *), const void *existing_holder = nullptr) { - if (!srcs.result.second) { + if (!srcs.result.tinfo) { // No pybind11 type info. Raise an exception. - std::string tname = srcs.downcast.type ? srcs.downcast.type->name() - : srcs.original.type ? srcs.original.type->name() - : ""; + std::string tname = srcs.downcast.cpptype ? srcs.downcast.cpptype->name() + : srcs.original.cpptype ? srcs.original.cpptype->name() + : ""; detail::clean_type_id(tname); std::string msg = "Unregistered type : " + tname; set_error(PyExc_TypeError, msg.c_str()); return handle(); } - void *src = const_cast(srcs.result.first); + void *src = const_cast(srcs.result.cppobj); if (src == nullptr) { return none().release(); } - const type_info *tinfo = srcs.result.second; + const type_info *tinfo = srcs.result.tinfo; if (handle registered_inst = find_registered_python_instance(src, tinfo)) { return registered_inst; @@ -1581,7 +1588,7 @@ struct polymorphic_type_hook : public polymorphic_type_hook_base {}; PYBIND11_NAMESPACE_BEGIN(detail) template -type_caster_generic::cast_sources::cast_sources(const itype *ptr) : original{ptr, &typeid(itype)} { +cast_sources::cast_sources(const itype *ptr) : original{ptr, &typeid(itype)} { // If this is a base pointer to a derived type, and the derived type is // registered with pybind11, we want to make the full derived object // available. In the typical case where itype is polymorphic, we get the @@ -1590,7 +1597,7 @@ type_caster_generic::cast_sources::cast_sources(const itype *ptr) : original{ptr // specialization of polymorphic_type_hook can do the same thing. // If there is no downcast to perform, then the default hook will leave // derived.type set to nullptr, which causes us to ignore derived.obj. - downcast.obj = polymorphic_type_hook::get(ptr, downcast.type); + downcast.cppobj = polymorphic_type_hook::get(ptr, downcast.cpptype); result = resolve(); } @@ -1605,9 +1612,12 @@ class type_caster_base : public type_caster_generic { type_caster_base() : type_caster_base(typeid(type)) {} explicit type_caster_base(const std::type_info &info) : type_caster_generic(info) {} - struct cast_sources : type_caster_generic::cast_sources { + // Wrap the generic cast_sources to be only constructible from the type + // that's correct in this context, so you can't use type_caster_base + // to convert an unrelated B* to Python. + struct cast_sources : detail::cast_sources { // NOLINTNEXTLINE(google-explicit-constructor) - cast_sources(const itype *ptr) : type_caster_generic::cast_sources(ptr) {} + cast_sources(const itype *ptr) : detail::cast_sources(ptr) {} }; static handle cast(const itype &src, return_value_policy policy, handle parent) { From 9e59f63eaf7038fbe9c0b1761ebf895192560338 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 14 Oct 2025 23:43:48 +0000 Subject: [PATCH 3/3] style: pre-commit fixes --- include/pybind11/detail/type_caster_base.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index d257f01d6f..e0b68ce73b 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -568,7 +568,7 @@ struct cast_sources { // this does not provide enough information to use a foreign type or // to render a useful error message cast_sources(const void *obj, const detail::type_info *tinfo) - : original{obj, tinfo ? tinfo->cpptype : nullptr}, result{obj, tinfo} {} + : original{obj, tinfo ? tinfo->cpptype : nullptr}, result{obj, tinfo} {} // The object passed to cast(), with its static type. // original.type must not be null if resolve() will be called. @@ -1009,7 +1009,7 @@ class type_caster_generic { // No pybind11 type info. Raise an exception. std::string tname = srcs.downcast.cpptype ? srcs.downcast.cpptype->name() : srcs.original.cpptype ? srcs.original.cpptype->name() - : ""; + : ""; detail::clean_type_id(tname); std::string msg = "Unregistered type : " + tname; set_error(PyExc_TypeError, msg.c_str());