Skip to content

Commit d930de0

Browse files
rwgkrwgk
authored
Alternative approach to #3807 (#4612)
* Alternative approach to #3807 that supports an important PyCLIF use case: `Base` not specified in `classh<Derived>` statement, but passing Derived as Base works anyway. * NOtest_multiple_inheritance_getattr in test_class_sh_void_ptr_capsule.py (quick experiment) * Revert "NOtest_multiple_inheritance_getattr in test_class_sh_void_ptr_capsule.py (quick experiment)" This reverts commit e8f0749. * Special handling of pybind11 objects to side-step try_as_void_ptr_capsule_get_pointer __getattr__ issues. * Inspect `internals.registered_types_py` in `type_is_pybind11_class_()` * Remove debug code in tests subdir. * Clean up the modified `try_as_void_ptr_capsule_get_pointer()` implementation and new helper functions.
1 parent c6a4191 commit d930de0

File tree

3 files changed

+83
-27
lines changed

3 files changed

+83
-27
lines changed

‎include/pybind11/detail/smart_holder_type_casters.h

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ struct is_smart_holder_type<smart_holder> : std::true_type {};
3636
// SMART_HOLDER_WIP: Needs refactoring of existing pybind11 code.
3737
inline void register_instance(instance *self, void *valptr, const type_info *tinfo);
3838
inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo);
39+
extern "C" inline PyObject *pybind11_object_new(PyTypeObject *type, PyObject *, PyObject *);
3940

4041
// Replace all occurrences of substrings in a string.
4142
inline void replace_all(std::string &str, const std::string &from, const std::string &to) {
@@ -49,20 +50,52 @@ inline void replace_all(std::string &str, const std::string &from, const std::st
4950
}
5051
}
5152

53+
inline bool type_is_pybind11_class_(PyTypeObject *type_obj) {
54+
#if defined(PYPY_VERSION)
55+
auto &internals = get_internals();
56+
return bool(internals.registered_types_py.find(type_obj)
57+
!= internals.registered_types_py.end());
58+
#else
59+
return bool(type_obj->tp_new == pybind11_object_new);
60+
#endif
61+
}
62+
63+
inline bool is_instance_method_of_type(PyTypeObject *type_obj, PyObject *attr_name) {
64+
PyObject *descr = _PyType_Lookup(type_obj, attr_name);
65+
return bool((descr != nullptr) && PyInstanceMethod_Check(descr));
66+
}
67+
68+
inline object try_get_as_capsule_method(PyObject *obj, PyObject *attr_name) {
69+
if (PyType_Check(obj)) {
70+
return object();
71+
}
72+
PyTypeObject *type_obj = Py_TYPE(obj);
73+
bool known_callable = false;
74+
if (type_is_pybind11_class_(type_obj)) {
75+
if (!is_instance_method_of_type(type_obj, attr_name)) {
76+
return object();
77+
}
78+
known_callable = true;
79+
}
80+
PyObject *method = PyObject_GetAttr(obj, attr_name);
81+
if (method == nullptr) {
82+
PyErr_Clear();
83+
return object();
84+
}
85+
if (!known_callable && PyCallable_Check(method) == 0) {
86+
Py_DECREF(method);
87+
return object();
88+
}
89+
return reinterpret_steal<object>(method);
90+
}
91+
5292
inline void *try_as_void_ptr_capsule_get_pointer(handle src, const char *typeid_name) {
53-
std::string type_name = typeid_name;
54-
detail::clean_type_id(type_name);
55-
56-
// Convert `a::b::c` to `a_b_c`.
57-
replace_all(type_name, "::", "_");
58-
// Remove all `*` in the type name.
59-
replace_all(type_name, "*", "");
60-
61-
std::string as_void_ptr_function_name("as_");
62-
as_void_ptr_function_name += type_name;
63-
if (hasattr(src, as_void_ptr_function_name.c_str())) {
64-
auto as_void_ptr_function = function(src.attr(as_void_ptr_function_name.c_str()));
65-
auto void_ptr_capsule = as_void_ptr_function();
93+
std::string suffix = clean_type_id(typeid_name);
94+
replace_all(suffix, "::", "_"); // Convert `a::b::c` to `a_b_c`.
95+
replace_all(suffix, "*", "");
96+
object as_capsule_method = try_get_as_capsule_method(src.ptr(), str("as_" + suffix).ptr());
97+
if (as_capsule_method) {
98+
object void_ptr_capsule = as_capsule_method();
6699
if (isinstance<capsule>(void_ptr_capsule)) {
67100
return reinterpret_borrow<capsule>(void_ptr_capsule).get_pointer();
68101
}
@@ -304,11 +337,8 @@ class modified_type_caster_generic_load_impl {
304337
loaded_v_h = value_and_holder();
305338
return true;
306339
}
307-
if (convert && cpptype) {
308-
const auto &bases = all_type_info(srctype);
309-
if (bases.empty() && try_as_void_ptr_capsule(src)) {
310-
return true;
311-
}
340+
if (convert && cpptype && try_as_void_ptr_capsule(src)) {
341+
return true;
312342
}
313343
return false;
314344
}

‎tests/test_class_sh_void_ptr_capsule.cpp

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,33 @@ struct Derived2 : Base12 {
5858
int bar() const { return 2; }
5959
};
6060

61+
struct UnspecBase {
62+
virtual ~UnspecBase() = default;
63+
virtual int Get() const { return 100; }
64+
};
65+
66+
int PassUnspecBase(const UnspecBase &sb) { return sb.Get() + 30; }
67+
68+
struct UnspecDerived : UnspecBase {
69+
int Get() const override { return 200; }
70+
};
71+
6172
} // namespace class_sh_void_ptr_capsule
6273
} // namespace pybind11_tests
6374

64-
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Valid)
65-
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::TypeWithGetattr)
66-
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Base1)
67-
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Base2)
68-
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Base12)
69-
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Derived1)
70-
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Derived2)
75+
using namespace pybind11_tests::class_sh_void_ptr_capsule;
7176

72-
TEST_SUBMODULE(class_sh_void_ptr_capsule, m) {
73-
using namespace pybind11_tests::class_sh_void_ptr_capsule;
77+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(Valid)
78+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(TypeWithGetattr)
79+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(Base1)
80+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(Base2)
81+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(Base12)
82+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(Derived1)
83+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(Derived2)
84+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(UnspecBase)
85+
PYBIND11_SMART_HOLDER_TYPE_CASTERS(UnspecDerived)
7486

87+
TEST_SUBMODULE(class_sh_void_ptr_capsule, m) {
7588
py::classh<Valid>(m, "Valid");
7689

7790
m.def("get_from_valid_capsule", &get_from_valid_capsule);
@@ -102,4 +115,13 @@ TEST_SUBMODULE(class_sh_void_ptr_capsule, m) {
102115
py::classh<Derived1, Base12>(m, "Derived1").def(py::init<>()).def("bar", &Derived1::bar);
103116

104117
py::classh<Derived2, Base12>(m, "Derived2").def(py::init<>()).def("bar", &Derived2::bar);
118+
119+
py::classh<UnspecBase>(m, "UnspecBase");
120+
m.def("PassUnspecBase", PassUnspecBase);
121+
py::classh<UnspecDerived>(m, "UnspecDerived") // UnspecBase NOT specified as base here.
122+
.def(py::init<>())
123+
.def("as_pybind11_tests_class_sh_void_ptr_capsule_UnspecBase", [](UnspecDerived *self) {
124+
return py::reinterpret_steal<py::object>(
125+
PyCapsule_New(static_cast<void *>(self), nullptr, nullptr));
126+
});
105127
}

‎tests/test_class_sh_void_ptr_capsule.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,7 @@ def test_multiple_inheritance_getattr():
9494
assert d2.foo() == 0
9595
assert d2.bar() == 2
9696
assert d2.prop2 == "Base GetAttr: prop2"
97+
98+
99+
def test_pass_unspecified_base():
100+
assert m.PassUnspecBase(m.UnspecDerived()) == 230

0 commit comments

Comments
 (0)