Skip to content

Commit 7b8b40a

Browse files
committed
Small tweaks and binary size reduction
* Remove recursion flag and extra typeinfo checks. * Change loader from member function pointer to a regular function pointer for improved portability. * Reduce binary size on clang.
1 parent c3bf66f commit 7b8b40a

File tree

3 files changed

+34
-41
lines changed

3 files changed

+34
-41
lines changed

include/pybind11/cast.h

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ struct type_info {
5151
std::vector<bool (*)(PyObject *, void *&)> *direct_conversions;
5252
buffer_info *(*get_buffer)(PyObject *, void *) = nullptr;
5353
void *get_buffer_data = nullptr;
54-
bool (type_caster_generic::*module_local_load)(handle, bool) = nullptr;
54+
void *(*module_local_load)(PyObject *, const type_info *) = nullptr;
5555
/* A simple type never occurs as a (direct or indirect) parent
5656
* of a class that makes use of multiple inheritance */
5757
bool simple_type : 1;
@@ -586,6 +586,8 @@ class type_caster_generic {
586586
PYBIND11_NOINLINE type_caster_generic(const std::type_info &type_info)
587587
: typeinfo(get_type_info(type_info)) { }
588588

589+
type_caster_generic(const type_info *typeinfo) : typeinfo(typeinfo) { }
590+
589591
bool load(handle src, bool convert) {
590592
return load_impl<type_caster_generic>(src, convert);
591593
}
@@ -663,8 +665,6 @@ class type_caster_generic {
663665
return inst.release();
664666
}
665667

666-
protected:
667-
668668
// Base methods for generic caster; there are overridden in copyable_holder_caster
669669
void load_value(value_and_holder &&v_h) {
670670
auto *&vptr = v_h.value_ptr();
@@ -694,35 +694,22 @@ class type_caster_generic {
694694
}
695695
void check_holder_compat() {}
696696

697-
// If loading failed with the type visible to this module see if we can load by looking to
698-
// other modules. There are two possibilities here:
699-
// - the input type is actually for another module's `py::module_local` type; we can try getting
700-
// the other module to load it.
701-
// - the input type is a globally-registered type, but we failed to load it because we have a module-local
702-
// override; we can replace our typeinfo with the global typeinfo and try loading again.
703-
PYBIND11_NOINLINE bool try_external_load(handle src) {
704-
allow_externals = true; // Don't recurse
705-
706-
str local_key("_pybind11_module_local_typeinfo");
707-
if (hasattr((PyObject *) Py_TYPE(src.ptr()), local_key)) {
708-
type_info *foreign_typeinfo = reinterpret_borrow<capsule>(getattr(src, local_key));
709-
710-
// Only consider this foreign loader if actually foreign
711-
if (foreign_typeinfo->module_local_load != &type_caster_generic::load) {
712-
typeinfo = foreign_typeinfo;
713-
return (this->*(foreign_typeinfo->module_local_load))(src, false);
714-
}
697+
/// Try to load with foreign typeinfo, if available. Used when there is no
698+
/// native typeinfo, or when the native one wasn't able to produce a value.
699+
PYBIND11_NOINLINE bool try_load_foreign_module_local(handle src) {
700+
constexpr auto *local_key = "_pybind11_module_local_typeinfo";
701+
const auto pytype = src.get_type();
702+
if (!hasattr(pytype, local_key))
715703
return false;
716-
}
717704

718-
// The type is not module_local; if the type we looked up is, try again with the global type
719-
if (typeinfo && typeinfo->module_local) {
720-
if (auto gtype = get_global_type_info(*typeinfo->cpptype)) {
721-
typeinfo = gtype;
722-
return load(src, false);
723-
}
724-
}
705+
type_info *foreign_typeinfo = reinterpret_borrow<capsule>(getattr(pytype, local_key));
706+
if (foreign_typeinfo == typeinfo)
707+
return false; // Only consider this foreign loader if actually foreign
725708

709+
if (auto result = foreign_typeinfo->module_local_load(src.ptr(), foreign_typeinfo)) {
710+
value = result;
711+
return true;
712+
}
726713
return false;
727714
}
728715

@@ -732,7 +719,7 @@ class type_caster_generic {
732719
template <typename ThisT>
733720
PYBIND11_NOINLINE bool load_impl(handle src, bool convert) {
734721
if (!src) return false;
735-
if (!typeinfo) return allow_externals && try_external_load(src);
722+
if (!typeinfo) return try_load_foreign_module_local(src);
736723
if (src.is_none()) {
737724
// Defer accepting None to other overloads (if we aren't in convert mode):
738725
if (!convert) return false;
@@ -798,7 +785,16 @@ class type_caster_generic {
798785
return true;
799786
}
800787

801-
return allow_externals && try_external_load(src);
788+
// Failed to match local typeinfo. Try again with global.
789+
if (typeinfo->module_local) {
790+
if (auto gtype = get_global_type_info(*typeinfo->cpptype)) {
791+
typeinfo = gtype;
792+
return load(src, false);
793+
}
794+
}
795+
796+
// Global typeinfo has precedence over foreign module_local
797+
return try_load_foreign_module_local(src);
802798
}
803799

804800

@@ -820,7 +816,6 @@ class type_caster_generic {
820816

821817
const type_info *typeinfo = nullptr;
822818
void *value = nullptr;
823-
bool allow_externals = true;
824819
};
825820

826821
/**

include/pybind11/detail/common.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,6 @@ enum class return_value_policy : uint8_t {
355355
reference_internal
356356
};
357357

358-
// forward declaration:
359-
class handle;
360-
361358
NAMESPACE_BEGIN(detail)
362359

363360
inline static constexpr int log2(size_t n, int k = 0) { return (n <= 1) ? k : log2(n >> 1, k + 1); }
@@ -488,9 +485,6 @@ struct type_equal_to {
488485
template <typename value_type>
489486
using type_map = std::unordered_map<std::type_index, value_type, type_hash, type_equal_to>;
490487

491-
// forward declaration
492-
class type_caster_generic;
493-
494488
/// Internal data structure used to track registered instances and types
495489
struct internals {
496490
type_map<void *> registered_types_cpp; // std::type_index -> type_info

include/pybind11/pybind11.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -868,9 +868,13 @@ class generic_type : public object {
868868
}
869869

870870
if (rec.module_local) {
871-
// Stash a capsule containing the local module's typeinfo so that external modules can
872-
// get at our type loader to try loading a C++ type from our module-local Python type.
873-
tinfo->module_local_load = &type_caster_generic::load;
871+
// Stash the local typeinfo and loader so that external modules can access it.
872+
tinfo->module_local_load = [](PyObject *src, const type_info *ti) -> void * {
873+
auto caster = type_caster_generic(ti);
874+
if (caster.load(src, false))
875+
return caster.value;
876+
return nullptr;
877+
};
874878
setattr(m_ptr, "_pybind11_module_local_typeinfo", capsule(tinfo));
875879
}
876880
}

0 commit comments

Comments
 (0)