From b3c9615d2de9e743ef5f2ab337d95430c11b0912 Mon Sep 17 00:00:00 2001 From: Arnim Balzer Date: Sat, 4 Mar 2023 07:25:24 +0000 Subject: [PATCH 1/2] Extend trampoline class pattern with example for nesting trampoline class hierarchies --- docs/advanced/classes.rst | 58 ++++++++++++++++++++++++ tests/test_multiple_inheritance.cpp | 68 +++++++++++++++++++++++++++++ tests/test_multiple_inheritance.py | 9 ++++ 3 files changed, 135 insertions(+) diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index 01a490b72..abde2c64f 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -903,6 +903,64 @@ are listed. .. _module_local: +For more complex multiple-inheritance class architectures with virtual methods, it might be necessary to combine +two different Trampoline class hierarchies. A templating trick can be used in this case to interleaf another +trampoline class (-hierarchy): + +.. code-block:: cpp + + class Animal { + public: + virtual ~Animal() { } + virtual std::string go(int n_times) = 0; + }; + template + class PyAnimal : public AnimalBase { + public: + using AnimalBase::AnimalBase; // Inherit constructors + std::string go(int n_times) override { PYBIND11_OVERRIDE_PURE(std::string, AnimalBase, go, n_times); } + }; + + class Dog : public Animal { + public: + std::string go(int n_times) override; + }; + template + class PyDog : public PyAnimal { + public: + using PyAnimal::PyAnimal; // Inherit constructors + std::string go(int n_times) override { PYBIND11_OVERRIDE(std::string, DogBase, go, n_times); } + }; + + class Mutant { + public: + virtual ~Mutant() { } + virtual void transform(); + }; + template + class PyMutant : public PyMutantBase { + public: + using PyMutantBase::PyMutantBase; // Inherit constructors + void transform() override { PYBIND11_OVERRIDE_PURE(void, MutantBase, transform, ); } + }; + + class Chimera : public Dog, public Mutant { + public: + virtual ~Chimera() { } + }; + template + class PyChimera : public PyMutant> { + public: + using PyMutant>::PyMutant; // Inherit constructors + }; + +The class ``Chimera`` inherits from both ``Dog`` and ``Mutant`` both of which feature virtual methods and +trampoline classes for binding. However, the ``Mutant`` trampoline class uses a second template parameter +``PyMutantBase`` so it can be injected into the single inheritance structure required by a trampoline class. +In effect, this mechanism enforces that the actual class the trampolines are using is only inherited from once. +Since the trampolines only need to add their respective trampoline function registrations, the order of the +inheritance of the various trampoline classes does not matter. + Module-local class bindings =========================== diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index 5916ae901..c70bb529e 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -80,6 +80,59 @@ struct I801D : I801C {}; // Indirect MI } // namespace +namespace TrampolineNesting { +class ChainBaseA { +public: + ChainBaseA() = default; + ChainBaseA(const ChainBaseA &) = default; + ChainBaseA(ChainBaseA &&) = default; + virtual ~ChainBaseA() = default; + virtual int resultA() { return 1; } +}; +class ChainChildA : public ChainBaseA { +public: + using ChainBaseA::ChainBaseA; + int resultA() override { return 2; } +}; +class ChainBaseB { +public: + ChainBaseB() = default; + ChainBaseB(const ChainBaseB &) = default; + ChainBaseB(ChainBaseB &&) = default; + virtual ~ChainBaseB() = default; + virtual std::string resultB() { return "A"; } +}; +class ChainChildB : public ChainBaseB { +public: + using ChainBaseB::ChainBaseB; + std::string resultB() override { return "B"; } +}; +class Joined : public ChainChildA, public ChainChildB { +public: + Joined() = default; + Joined(const Joined &) = default; + Joined(Joined &&) = default; +}; + +template +class TrampolineA : public Base { +public: + using Base::Base; + int resultA() override { PYBIND11_OVERLOAD(int, Base, resultA, ); } +}; +template +class TrampolineB : public PyBase { +public: + using PyBase::PyBase; + std::string resultB() override { PYBIND11_OVERLOAD(std::string, Base, resultB, ); } +}; +template +class TrampolineJoined : public TrampolineB> { +public: + using TrampolineB>::TrampolineB; +}; +} // namespace TrampolineNesting + TEST_SUBMODULE(multiple_inheritance, m) { // Please do not interleave `struct` and `class` definitions with bindings code, // but implement `struct`s and `class`es in the anonymous namespace above. @@ -338,4 +391,19 @@ TEST_SUBMODULE(multiple_inheritance, m) { .def("get_f_e", &MVF::get_f_e) .def("get_f_f", &MVF::get_f_f) .def_readwrite("f", &MVF::f); + + namespace TN = TrampolineNesting; + + py::class_>(m, "ChainBaseA") + .def(py::init<>()) + .def("resultA", &TN::ChainBaseA::resultA); + py::class_>(m, "ChainChildA") + .def(py::init<>()); + py::class_>(m, "ChainBaseB") + .def(py::init<>()) + .def("resultB", &TN::ChainBaseB::resultB); + py::class_>(m, "ChainChildB") + .def(py::init<>()); + py::class_>(m, "Joined") + .def(py::init<>()); } diff --git a/tests/test_multiple_inheritance.py b/tests/test_multiple_inheritance.py index 3a1d88d71..e6acc2939 100644 --- a/tests/test_multiple_inheritance.py +++ b/tests/test_multiple_inheritance.py @@ -491,3 +491,12 @@ def test_python_inherit_from_mi(): assert o.g == 7 assert o.get_g_g() == 7 + + +def test_trampoline_nesting(): + assert m.ChainBaseA().resultA() == 1 + assert m.ChainChildA().resultA() == 2 + assert m.ChainBaseB().resultB() == "A" + assert m.ChainChildB().resultB() == "B" + assert m.Joined().resultA() == 2 + assert m.Joined().resultB() == "B" From cbb0590ac7dd65232d89d6dbdc3b0ce01ff589dd Mon Sep 17 00:00:00 2001 From: Arnim Balzer Date: Sat, 4 Mar 2023 09:53:10 +0000 Subject: [PATCH 2/2] Add better support for pure virtual methods --- docs/advanced/classes.rst | 59 +++++++++++++++++++++++++++++ include/pybind11/pybind11.h | 46 ++++++++++++++++++++++ tests/test_multiple_inheritance.cpp | 33 +++++++++------- tests/test_multiple_inheritance.py | 14 ++++--- 4 files changed, 133 insertions(+), 19 deletions(-) diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index abde2c64f..bc9fe4337 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -867,6 +867,8 @@ which should look as follows: .. [#f5] https://docs.python.org/3/library/copy.html +.. _multiple_inheritance: + Multiple Inheritance ==================== @@ -961,6 +963,63 @@ In effect, this mechanism enforces that the actual class the trampolines are usi Since the trampolines only need to add their respective trampoline function registrations, the order of the inheritance of the various trampoline classes does not matter. +If the base classes contain pure virtual methods, another pattern can be applied to reduce the amount of +trampoline code that needs writing. The cost is an additional ``std::same`` call for each pure-virtual +method using the macro ``PYBIND11_OVERRIDE_TEMPLATE``. + +.. code-block:: cpp + + class Animal { + public: + virtual ~Animal() { } + virtual std::string go(int n_times) = 0; + }; + class Dog : public Animal { + public: + std::string go(int n_times) override; + }; + template + class PyAnimal : public AnimalBase { + public: + using AnimalBase::AnimalBase; // Inherit constructors + std::string go(int n_times) override { PYBIND11_OVERRIDE_TEMPLATE(PureVirtualBase, std::string, AnimalBase, go, n_times); } + }; + using PyDog = PyAnimal + + class Mutant { + public: + virtual ~Mutant() { } + virtual void transform() = 0; + }; + class XMen : public Mutant{ + public: + virtual ~Mutant() { } + void transform() override; + }; + template + class PyMutant : public PyMutantBase { + public: + using PyMutantBase::PyMutantBase; // Inherit constructors + void transform() override { PYBIND11_OVERRIDE_TEMPLATE(PureVirtualBase, void, MutantBase, transform, ); } + }; + using PyXMen = PyMutant + + class Chimera : public Dog, public Mutant { + public: + virtual ~Chimera() { } + }; + template + class PyChimera : public PyMutant> { + public: + using PyMutant>::PyMutant; // Inherit constructors + }; + +The first parameter of the :c:macro:`PYBIND11_OVERRIDE_TEMPLATE` is the base class containing +the pure virtual method. Together with the cname parameter, an ``std::same`` call is used to +invoke either :c:macro:`PYBIND11_OVERRIDE_PURE` or :c:macro:`PYBIND11_OVERRIDE`. A corresponding +:c:macro:`PYBIND11_OVERRIDE_TEMPLATE_NAME` implementation is also available. The template parameter +``PureVirtualBase`` can be used in case the pure virtual methods are not implemented in a child class. + Module-local class bindings =========================== diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 6205effd6..e9d08b5ac 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -2854,6 +2855,51 @@ function get_override(const T *this_ptr, const char *name) { PYBIND11_OVERRIDE_PURE_NAME( \ PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), #fn, fn, __VA_ARGS__) +/** \rst + Macro to wrap :c:macro:`PYBIND11_OVERRIDE_PURE_NAME` and :c:macro:`PYBIND11_OVERRIDE_PURE` + depending on the base class and cname parameter provided. + See :ref:`_multiple_inheritance` for more information. + + .. code-block:: cpp + + template + class PyAnimal : public AnimalBase { + public: + // Inherit the constructors + using AnimalBase::AnimalBase; + + // Trampoline (need one for each virtual function) + std::string go(int n_times) override { + PYBIND11_OVERRIDE_TEMPLATE( + Animal, // The base class containing the purely virtual implementation + std::string, // Return type (ret_type) + Dog, // Parent class (cname) + "_go", // Name of method in Python (name) + go, // Name of function in C++ (must match Python name) (fn) + n_times // Argument(s) (...) + ); + } + }; +\endrst */ +#define PYBIND11_OVERRIDE_TEMPLATE_NAME(base, ret_type, cname, name, fn, ...) \ + if (std::is_same::value) { \ + PYBIND11_OVERRIDE_PURE_NAME( \ + PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), name, fn, __VA_ARGS__); \ + } else { \ + PYBIND11_OVERRIDE_NAME( \ + PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), name, fn, __VA_ARGS__); \ + } + +/** \rst + Macro to wrap :c:macro:`PYBIND11_OVERRIDE_NAME` and :c:macro:`PYBIND11_OVERRIDE` + depending on the base class and cname parameter provided. + Uses :c:macro:`PYBIND11_OVERRIDE_TEMPLATE_NAME` under the hood. + See :ref:`_multiple_inheritance` for more information. +\endrst */ +#define PYBIND11_OVERRIDE_TEMPLATE(base, ret_type, cname, fn, ...) \ + PYBIND11_OVERRIDE_TEMPLATE_NAME( \ + PYBIND11_TYPE(base), PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), #fn, fn, __VA_ARGS__) + // Deprecated versions PYBIND11_DEPRECATED("get_type_overload has been deprecated") diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index c70bb529e..acca5efa2 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -87,12 +87,12 @@ public: ChainBaseA(const ChainBaseA &) = default; ChainBaseA(ChainBaseA &&) = default; virtual ~ChainBaseA() = default; - virtual int resultA() { return 1; } + virtual int resultA() = 0; }; class ChainChildA : public ChainBaseA { public: using ChainBaseA::ChainBaseA; - int resultA() override { return 2; } + int resultA() override { return 1; } }; class ChainBaseB { public: @@ -100,12 +100,12 @@ public: ChainBaseB(const ChainBaseB &) = default; ChainBaseB(ChainBaseB &&) = default; virtual ~ChainBaseB() = default; - virtual std::string resultB() { return "A"; } + virtual std::string resultB() = 0; }; class ChainChildB : public ChainBaseB { public: using ChainBaseB::ChainBaseB; - std::string resultB() override { return "B"; } + std::string resultB() override { return "A"; } }; class Joined : public ChainChildA, public ChainChildB { public: @@ -114,22 +114,24 @@ public: Joined(Joined &&) = default; }; -template +template class TrampolineA : public Base { public: using Base::Base; - int resultA() override { PYBIND11_OVERLOAD(int, Base, resultA, ); } + int resultA() override { PYBIND11_OVERRIDE_TEMPLATE(PureVirtualBase, int, Base, resultA, ) } }; -template +template class TrampolineB : public PyBase { public: using PyBase::PyBase; - std::string resultB() override { PYBIND11_OVERLOAD(std::string, Base, resultB, ); } + std::string resultB() override { + PYBIND11_OVERRIDE_TEMPLATE(PureVirtualBase, std::string, Base, resultB, ) + } }; -template -class TrampolineJoined : public TrampolineB> { +template +class TrampolineJoined : public TrampolineB> { public: - using TrampolineB>::TrampolineB; + using TrampolineB>::TrampolineB; }; } // namespace TrampolineNesting @@ -393,7 +395,6 @@ TEST_SUBMODULE(multiple_inheritance, m) { .def_readwrite("f", &MVF::f); namespace TN = TrampolineNesting; - py::class_>(m, "ChainBaseA") .def(py::init<>()) .def("resultA", &TN::ChainBaseA::resultA); @@ -404,6 +405,12 @@ TEST_SUBMODULE(multiple_inheritance, m) { .def("resultB", &TN::ChainBaseB::resultB); py::class_>(m, "ChainChildB") .def(py::init<>()); - py::class_>(m, "Joined") + py::class_>(m, "Joined") .def(py::init<>()); } + +// Needed for MSVC linker +namespace TrampolineNesting { +int ChainBaseA::resultA() { return 0; } +std::string ChainBaseB::resultB() { return ""; } +} // namespace TrampolineNesting diff --git a/tests/test_multiple_inheritance.py b/tests/test_multiple_inheritance.py index e6acc2939..497307a8b 100644 --- a/tests/test_multiple_inheritance.py +++ b/tests/test_multiple_inheritance.py @@ -494,9 +494,11 @@ def test_python_inherit_from_mi(): def test_trampoline_nesting(): - assert m.ChainBaseA().resultA() == 1 - assert m.ChainChildA().resultA() == 2 - assert m.ChainBaseB().resultB() == "A" - assert m.ChainChildB().resultB() == "B" - assert m.Joined().resultA() == 2 - assert m.Joined().resultB() == "B" + with pytest.raises(RuntimeError): + m.ChainBaseA().resultA() + assert m.ChainChildA().resultA() == 1 + with pytest.raises(RuntimeError): + m.ChainBaseB().resultB() + assert m.ChainChildB().resultB() == "A" + assert m.Joined().resultA() == 1 + assert m.Joined().resultB() == "A"