From b961626c0c2132dde5ec81036fc9f83398d95f30 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 16 Mar 2017 20:10:48 -0300 Subject: [PATCH] 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_ 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. --- include/pybind11/attr.h | 2 +- include/pybind11/pybind11.h | 20 +++++++++++++++----- tests/test_multiple_inheritance.cpp | 21 +++++++++++++++------ 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 7c3b84c8..450a55d5 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -333,7 +333,7 @@ template <> struct process_attribute : process_attribute_default { } }; -/// Process a parent class attribute +/// Process a parent class attribute. Single inheritance only (class_ itself already guarantees that) template struct process_attribute::value>> : process_attribute_default { static void init(const handle &h, type_record *r) { r->bases.append(h); } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 05b2b9e2..e90c69ae 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -885,11 +885,21 @@ public: template class_(handle scope, const char *name, const Extra &... extra) { - detail::type_record record; + using namespace detail; + + // MI can only be specified via class_ template options, not constructor parameters + static_assert( + none_of...>::value || // no base class arguments, or: + ( constexpr_sum(is_pyobject::value...) == 1 && // Exactly one base + constexpr_sum(is_base::value...) == 0 && // no template option bases + none_of...>::value), // no multiple_inheritance attr + "Error: multiple inheritance bases must be specified via class_ template options"); + + type_record record; record.scope = scope; record.name = name; record.type = &typeid(type); - record.type_size = sizeof(detail::conditional_t); + record.type_size = sizeof(conditional_t); record.instance_size = sizeof(instance_type); record.init_holder = init_holder; record.dealloc = dealloc; @@ -900,12 +910,12 @@ public: (void) unused; /* Process optional arguments, if any */ - detail::process_attributes::init(extra..., &record); + process_attributes::init(extra..., &record); - detail::generic_type::initialize(record); + generic_type::initialize(record); if (has_alias) { - auto &instances = pybind11::detail::get_internals().registered_types_cpp; + auto &instances = get_internals().registered_types_cpp; instances[std::type_index(typeid(type_alias))] = instances[std::type_index(typeid(type))]; } } diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index ef26cd84..3ebeb202 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -31,18 +31,27 @@ struct MIType : Base12 { }; test_initializer multiple_inheritance([](py::module &m) { - py::class_(m, "Base1") - .def(py::init()) - .def("foo", &Base1::foo); + py::class_ b1(m, "Base1"); + b1.def(py::init()) + .def("foo", &Base1::foo); - py::class_(m, "Base2") - .def(py::init()) - .def("bar", &Base2::bar); + py::class_ b2(m, "Base2"); + b2.def(py::init()) + .def("bar", &Base2::bar); py::class_(m, "Base12"); py::class_(m, "MIType") .def(py::init()); + + // Uncommenting this should result in a compile time failure (MI can only be specified via + // template parameters because pybind has to know the types involved; see discussion in #742 for + // details). +// struct Base12v2 : Base1, Base2 { +// Base12v2(int i, int j) : Base1(i), Base2(j) { } +// }; +// py::class_(m, "Base12v2", b1, b2) +// .def(py::init()); }); /* Test the case where not all base classes are specified,