Override deduced Base class when defining Derived methods
When defining method from a member function pointer (e.g. `.def("f",
&Derived::f)`) we run into a problem if `&Derived::f` is actually
implemented in some base class `Base` when `Base` isn't
pybind-registered.
This happens because the class type is deduced from the member function
pointer, which then becomes a lambda with first argument this deduced
type. For a base class implementation, the deduced type is `Base`, not
`Derived`, and so we generate and registered an overload which takes a
`Base *` as first argument. Trying to call this fails if `Base` isn't
registered (e.g. because it's an implementation detail class that isn't
intended to be exposed to Python) because the type caster for an
unregistered type always fails.
This commit adds a `method_adaptor` function that rebinds a member
function to a derived type member function and otherwise (i.e. regular
functions/lambda) leaves the argument as-is. This is now used for class
definitions so that they are bound with type being registered rather
than a potential base type.
A closely related fix in this commit is to similarly update the lambdas
used for `def_readwrite` (and related) to bind to the class type being
registered rather than the deduced type so that registering a property
that resolves to a base class member similarly generates a usable
function.
Fixes #854, #910.
Co-Authored-By: Dean Moldovan <dean0x7d@gmail.com>
diff --git a/include/pybind11/common.h b/include/pybind11/common.h
index 1a5f7fa..ed1bb61 100644
--- a/include/pybind11/common.h
+++ b/include/pybind11/common.h
@@ -620,6 +620,11 @@
template <typename T, typename... /*Us*/> struct deferred_type { using type = T; };
template <typename T, typename... Us> using deferred_t = typename deferred_type<T, Us...>::type;
+/// Like is_base_of, but requires a strict base (i.e. `is_strict_base_of<T, T>::value == false`,
+/// unlike `std::is_base_of`)
+template <typename Base, typename Derived> using is_strict_base_of = bool_constant<
+ std::is_base_of<Base, Derived>::value && !std::is_same<Base, Derived>::value>;
+
template <template<typename...> class Base>
struct is_template_base_of_impl {
template <typename... Us> static std::true_type check(Base<Us...> *);
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index e922903..6084d96 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -910,11 +910,22 @@
NAMESPACE_END(detail)
+/// Given a pointer to a member function, cast it to its `Derived` version.
+/// Forward everything else unchanged.
+template <typename /*Derived*/, typename F>
+auto method_adaptor(F &&f) -> decltype(std::forward<F>(f)) { return std::forward<F>(f); }
+
+template <typename Derived, typename Return, typename Class, typename... Args>
+auto method_adaptor(Return (Class::*pmf)(Args...)) -> Return (Derived::*)(Args...) { return pmf; }
+
+template <typename Derived, typename Return, typename Class, typename... Args>
+auto method_adaptor(Return (Class::*pmf)(Args...) const) -> Return (Derived::*)(Args...) const { return pmf; }
+
template <typename type_, typename... options>
class class_ : public detail::generic_type {
template <typename T> using is_holder = detail::is_holder_type<type_, T>;
- template <typename T> using is_subtype = detail::bool_constant<std::is_base_of<type_, T>::value && !std::is_same<T, type_>::value>;
- template <typename T> using is_base = detail::bool_constant<std::is_base_of<T, type_>::value && !std::is_same<T, type_>::value>;
+ template <typename T> using is_subtype = detail::is_strict_base_of<type_, T>;
+ template <typename T> using is_base = detail::is_strict_base_of<T, type_>;
// struct instead of using here to help MSVC:
template <typename T> struct is_valid_class_option :
detail::any_of<is_holder<T>, is_subtype<T>, is_base<T>> {};
@@ -980,7 +991,7 @@
template <typename Func, typename... Extra>
class_ &def(const char *name_, Func&& f, const Extra&... extra) {
- cpp_function cf(std::forward<Func>(f), name(name_), is_method(*this),
+ cpp_function cf(method_adaptor<type>(std::forward<Func>(f)), name(name_), is_method(*this),
sibling(getattr(*this, name_, none())), extra...);
attr(cf.name()) = cf;
return *this;
@@ -1044,15 +1055,17 @@
template <typename C, typename D, typename... Extra>
class_ &def_readwrite(const char *name, D C::*pm, const Extra&... extra) {
- cpp_function fget([pm](const C &c) -> const D &{ return c.*pm; }, is_method(*this)),
- fset([pm](C &c, const D &value) { c.*pm = value; }, is_method(*this));
+ static_assert(std::is_base_of<C, type>::value, "def_readwrite() requires a class member (or base class member)");
+ cpp_function fget([pm](const type &c) -> const D &{ return c.*pm; }, is_method(*this)),
+ fset([pm](type &c, const D &value) { c.*pm = value; }, is_method(*this));
def_property(name, fget, fset, return_value_policy::reference_internal, extra...);
return *this;
}
template <typename C, typename D, typename... Extra>
class_ &def_readonly(const char *name, const D C::*pm, const Extra& ...extra) {
- cpp_function fget([pm](const C &c) -> const D &{ return c.*pm; }, is_method(*this));
+ static_assert(std::is_base_of<C, type>::value, "def_readonly() requires a class member (or base class member)");
+ cpp_function fget([pm](const type &c) -> const D &{ return c.*pm; }, is_method(*this));
def_property_readonly(name, fget, return_value_policy::reference_internal, extra...);
return *this;
}
@@ -1075,7 +1088,8 @@
/// Uses return_value_policy::reference_internal by default
template <typename Getter, typename... Extra>
class_ &def_property_readonly(const char *name, const Getter &fget, const Extra& ...extra) {
- return def_property_readonly(name, cpp_function(fget), return_value_policy::reference_internal, extra...);
+ return def_property_readonly(name, cpp_function(method_adaptor<type>(fget)),
+ return_value_policy::reference_internal, extra...);
}
/// Uses cpp_function's return_value_policy by default
@@ -1097,9 +1111,14 @@
}
/// Uses return_value_policy::reference_internal by default
+ template <typename Getter, typename Setter, typename... Extra>
+ class_ &def_property(const char *name, const Getter &fget, const Setter &fset, const Extra& ...extra) {
+ return def_property(name, fget, cpp_function(method_adaptor<type>(fset)), extra...);
+ }
template <typename Getter, typename... Extra>
class_ &def_property(const char *name, const Getter &fget, const cpp_function &fset, const Extra& ...extra) {
- return def_property(name, cpp_function(fget), fset, return_value_policy::reference_internal, extra...);
+ return def_property(name, cpp_function(method_adaptor<type>(fget)), fset,
+ return_value_policy::reference_internal, extra...);
}
/// Uses cpp_function's return_value_policy by default
diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp
index 670f6c3..e8e1b52 100644
--- a/tests/test_methods_and_attributes.cpp
+++ b/tests/test_methods_and_attributes.cpp
@@ -159,7 +159,7 @@
};
}}
-/// Issue/PR #648: bad arg default debugging output
+// Issue/PR #648: bad arg default debugging output
class NotRegistered {};
// Test None-allowed py::arg argument policy
@@ -177,6 +177,23 @@
StrIssue(int i) : val{i} {}
};
+// Issues #854, #910: incompatible function args when member function/pointer is in unregistered base class
+class UnregisteredBase {
+public:
+ void do_nothing() const {}
+ void increase_value() { rw_value++; ro_value += 0.25; }
+ void set_int(int v) { rw_value = v; }
+ int get_int() const { return rw_value; }
+ double get_double() const { return ro_value; }
+ int rw_value = 42;
+ double ro_value = 1.25;
+};
+class RegisteredDerived : public UnregisteredBase {
+public:
+ using UnregisteredBase::UnregisteredBase;
+ double sum() const { return rw_value + ro_value; }
+};
+
test_initializer methods_and_attributes([](py::module &m) {
py::class_<ExampleMandA> emna(m, "ExampleMandA");
emna.def(py::init<>())
@@ -325,7 +342,7 @@
m.def("ints_preferred", [](int i) { return i / 2; }, py::arg("i"));
m.def("ints_only", [](int i) { return i / 2; }, py::arg("i").noconvert());
- /// Issue/PR #648: bad arg default debugging output
+ // Issue/PR #648: bad arg default debugging output
#if !defined(NDEBUG)
m.attr("debug_enabled") = true;
#else
@@ -360,4 +377,26 @@
.def("__str__", [](const StrIssue &si) {
return "StrIssue[" + std::to_string(si.val) + "]"; }
);
+
+ // Issues #854/910: incompatible function args when member function/pointer is in unregistered
+ // base class The methods and member pointers below actually resolve to members/pointers in
+ // UnregisteredBase; before this test/fix they would be registered via lambda with a first
+ // argument of an unregistered type, and thus uncallable.
+ py::class_<RegisteredDerived>(m, "RegisteredDerived")
+ .def(py::init<>())
+ .def("do_nothing", &RegisteredDerived::do_nothing)
+ .def("increase_value", &RegisteredDerived::increase_value)
+ .def_readwrite("rw_value", &RegisteredDerived::rw_value)
+ .def_readonly("ro_value", &RegisteredDerived::ro_value)
+ // These should trigger a static_assert if uncommented
+ //.def_readwrite("fails", &SimpleValue::value) // should trigger a static_assert if uncommented
+ //.def_readonly("fails", &SimpleValue::value) // should trigger a static_assert if uncommented
+ .def_property("rw_value_prop", &RegisteredDerived::get_int, &RegisteredDerived::set_int)
+ .def_property_readonly("ro_value_prop", &RegisteredDerived::get_double)
+ // This one is in the registered class:
+ .def("sum", &RegisteredDerived::sum)
+ ;
+
+ using Adapted = decltype(py::method_adaptor<RegisteredDerived>(&RegisteredDerived::do_nothing));
+ static_assert(std::is_same<Adapted, void (RegisteredDerived::*)() const>::value, "");
});
diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py
index 95049cf..afe8d28 100644
--- a/tests/test_methods_and_attributes.py
+++ b/tests/test_methods_and_attributes.py
@@ -457,3 +457,23 @@
Invoked with: 'no', 'such', 'constructor'
"""
+
+
+def test_unregistered_base_implementations():
+ from pybind11_tests import RegisteredDerived
+
+ a = RegisteredDerived()
+ a.do_nothing()
+ assert a.rw_value == 42
+ assert a.ro_value == 1.25
+ a.rw_value += 5
+ assert a.sum() == 48.25
+ a.increase_value()
+ assert a.rw_value == 48
+ assert a.ro_value == 1.5
+ assert a.sum() == 49.5
+ assert a.rw_value_prop == 48
+ a.rw_value_prop += 1
+ assert a.rw_value_prop == 49
+ a.increase_value()
+ assert a.ro_value_prop == 1.75