Skip to content

Commit 26cd9f0

Browse files
committed
Fail to compile with MI via class_ ctor parameters
We can't support this for classes from imported modules (which is the primary purpose of a ctor argument base class) because we *have* to have both parent and derived to properly extract a multiple-inheritance base class pointer from a derived class pointer. We could support this for actual `class_<Base, ...> instances, but since in that case the `Base` is already present in the code, it seems more consistent to simply always require MI to go via template options. This also adds a new "number_of" template, mostly based on the `count_t` removed in PR pybind#554, to count the number of satisfied templates; this is basically just `constexpr_sum`, and uses that except for under MSVC which doesn't think the constexpr is a constexpr.
1 parent 819cb55 commit 26cd9f0

File tree

4 files changed

+42
-12
lines changed

4 files changed

+42
-12
lines changed

include/pybind11/attr.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ template <> struct process_attribute<arg_v> : process_attribute_default<arg_v> {
333333
}
334334
};
335335

336-
/// Process a parent class attribute
336+
/// Process a parent class attribute. Single inheritance only (class_ itself already guarantees that)
337337
template <typename T>
338338
struct process_attribute<T, enable_if_t<is_pyobject<T>::value>> : process_attribute_default<handle> {
339339
static void init(const handle &h, type_record *r) { r->bases.append(h); }

include/pybind11/common.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,16 @@ constexpr size_t constexpr_sum() { return 0; }
452452
template <typename T, typename... Ts>
453453
constexpr size_t constexpr_sum(T n, Ts... ns) { return size_t{n} + constexpr_sum(ns...); }
454454

455+
/// Returns the number of the given template types with true ::values
456+
#if !defined(_MSC_VER)
457+
template <class... Ts> using number_of = std::integral_constant<size_t, constexpr_sum((size_t) (bool) Ts::value...)>;
458+
#else
459+
// MSVC workaround (2015 Update 3 has issues with some member type aliases and the above constexpr)
460+
template <class... Ts> struct number_of : std::integral_constant<size_t, 0> {};
461+
template <class T, class... Ts> struct number_of<T, Ts...>
462+
: std::integral_constant<size_t, (bool) Ts::value + number_of<Ts...>::value> {};
463+
#endif
464+
455465
NAMESPACE_BEGIN(constexpr_impl)
456466
/// Implementation details for constexpr functions
457467
constexpr int first(int i) { return i; }

include/pybind11/pybind11.h

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -885,11 +885,21 @@ class class_ : public detail::generic_type {
885885

886886
template <typename... Extra>
887887
class_(handle scope, const char *name, const Extra &... extra) {
888-
detail::type_record record;
888+
using namespace detail;
889+
890+
// MI can only be specified via class_ template options, not constructor parameters
891+
static_assert(
892+
none_of<is_pyobject<Extra>...>::value || // no base class arguments, or:
893+
(number_of<is_pyobject<Extra>...>::value == 1 && // Exactly one base
894+
none_of<is_base<options>...>::value && // no template option bases
895+
none_of<std::is_same<multiple_inheritance, Extra>...>::value), // no multiple_inheritance attr
896+
"Error: multiple inheritance bases must be specified via class_ template options");
897+
898+
type_record record;
889899
record.scope = scope;
890900
record.name = name;
891901
record.type = &typeid(type);
892-
record.type_size = sizeof(detail::conditional_t<has_alias, type_alias, type>);
902+
record.type_size = sizeof(conditional_t<has_alias, type_alias, type>);
893903
record.instance_size = sizeof(instance_type);
894904
record.init_holder = init_holder;
895905
record.dealloc = dealloc;
@@ -900,12 +910,12 @@ class class_ : public detail::generic_type {
900910
(void) unused;
901911

902912
/* Process optional arguments, if any */
903-
detail::process_attributes<Extra...>::init(extra..., &record);
913+
process_attributes<Extra...>::init(extra..., &record);
904914

905-
detail::generic_type::initialize(record);
915+
generic_type::initialize(record);
906916

907917
if (has_alias) {
908-
auto &instances = pybind11::detail::get_internals().registered_types_cpp;
918+
auto &instances = get_internals().registered_types_cpp;
909919
instances[std::type_index(typeid(type_alias))] = instances[std::type_index(typeid(type))];
910920
}
911921
}

tests/test_multiple_inheritance.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,33 @@ struct Base12 : Base1, Base2 {
2626
Base12(int i, int j) : Base1(i), Base2(j) { }
2727
};
2828

29+
struct Base12v2 : Base1, Base2 {
30+
Base12v2(int i, int j) : Base1(i), Base2(j) { }
31+
};
32+
2933
struct MIType : Base12 {
3034
MIType(int i, int j) : Base12(i, j) { }
3135
};
3236

3337
test_initializer multiple_inheritance([](py::module &m) {
34-
py::class_<Base1>(m, "Base1")
35-
.def(py::init<int>())
36-
.def("foo", &Base1::foo);
38+
py::class_<Base1> b1(m, "Base1");
39+
b1.def(py::init<int>())
40+
.def("foo", &Base1::foo);
3741

38-
py::class_<Base2>(m, "Base2")
39-
.def(py::init<int>())
40-
.def("bar", &Base2::bar);
42+
py::class_<Base2> b2(m, "Base2");
43+
b2.def(py::init<int>())
44+
.def("bar", &Base2::bar);
4145

4246
py::class_<Base12, Base1, Base2>(m, "Base12");
4347

4448
py::class_<MIType, Base12>(m, "MIType")
4549
.def(py::init<int, int>());
50+
51+
// Uncommenting this should result in a compile time failure (MI can only be specified via
52+
// template parameters because pybind has to know the types involved; see discussion in #742 for
53+
// details).
54+
// py::class_<Base12v2>(m, "Base12v2", b1, b2)
55+
// .def(py::init<int, int>());
4656
});
4757

4858
/* Test the case where not all base classes are specified,

0 commit comments

Comments
 (0)