diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 5634c355e..86c54ee81 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -116,8 +116,6 @@ enum op_id : int; enum op_type : int; struct undefined_t; template struct op_; -template struct init; -template struct init_alias; inline void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret); /// Internal data structure which holds metadata about a keyword argument diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index cefb32b41..ebbf94745 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -40,7 +40,7 @@ PYBIND11_NOINLINE inline value_and_holder load_v_h(handle self_, type_info *tinf } -// Implementing functions for py::init(...) +// Implementing functions for all forms of py::init<...> and py::init(...) template using Cpp = typename Class::type; template using Alias = typename Class::type_alias; template using Holder = typename Class::holder_type; @@ -161,6 +161,62 @@ void construct(value_and_holder &v_h, Alias &&result, bool) { deallocate(v_h) = new Alias(std::move(result)); } +// Implementing class for py::init<...>() +template struct constructor { + template = 0> + static void execute(Class &cl, const Extra&... extra) { + auto *cl_type = get_type_info(typeid(Cpp)); + cl.def("__init__", [cl_type](handle self_, Args... args) { + auto v_h = load_v_h(self_, cl_type); + if (v_h.instance_registered()) return; // Ignore duplicate __init__ calls (see above) + + construct(v_h, new Cpp(std::forward(args)...), false); + }, extra...); + } + + template , Args...>::value, int> = 0> + static void execute(Class &cl, const Extra&... extra) { + auto *cl_type = get_type_info(typeid(Cpp)); + cl.def("__init__", [cl_type](handle self_, Args... args) { + auto v_h = load_v_h(self_, cl_type); + if (v_h.instance_registered()) return; // Ignore duplicate __init__ calls (see above) + + if (Py_TYPE(v_h.inst) == cl_type->type) + construct(v_h, new Cpp(std::forward(args)...), false); + else + construct(v_h, new Alias(std::forward(args)...), true); + }, extra...); + } + + template , Args...>::value, int> = 0> + static void execute(Class &cl, const Extra&... extra) { + auto *cl_type = get_type_info(typeid(Cpp)); + cl.def("__init__", [cl_type](handle self_, Args... args) { + auto v_h = load_v_h(self_, cl_type); + if (v_h.instance_registered()) return; // Ignore duplicate __init__ calls (see above) + construct(v_h, new Alias(std::forward(args)...), true); + }, extra...); + } +}; + +// Implementing class for py::init_alias<...>() +template struct alias_constructor { + template , Args...>::value, int> = 0> + static void execute(Class &cl, const Extra&... extra) { + auto *cl_type = get_type_info(typeid(Cpp)); + cl.def("__init__", [cl_type](handle self_, Args... args) { + auto v_h = load_v_h(self_, cl_type); + if (v_h.instance_registered()) return; // Ignore duplicate __init__ calls (see above) + construct(v_h, new Alias(std::forward(args)...), true); + }, extra...); + } +}; + // Implementation class for py::init(Func) and py::init(Func, AliasFunc) template struct factory { private: diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index d168b67be..afa91accd 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1046,13 +1046,13 @@ public: } template - class_ &def(const detail::init &init, const Extra&... extra) { + class_ &def(const detail::initimpl::constructor &init, const Extra&... extra) { init.execute(*this, extra...); return *this; } template - class_ &def(const detail::init_alias &init, const Extra&... extra) { + class_ &def(const detail::initimpl::alias_constructor &init, const Extra&... extra) { init.execute(*this, extra...); return *this; } @@ -1351,10 +1351,10 @@ private: }; /// Binds an existing constructor taking arguments Args... -template detail::init init() { return detail::init(); } +template detail::initimpl::constructor init() { return {}; } /// Like `init()`, but the instance is always constructed through the alias class (even /// when not inheriting on the Python side). -template detail::init_alias init_alias() { return detail::init_alias(); } +template detail::initimpl::alias_constructor init_alias() { return {}; } /// Binds a factory function as a constructor template > @@ -1368,44 +1368,6 @@ Ret init(CFunc &&c, AFunc &&a) { } NAMESPACE_BEGIN(detail) -template struct init { - template = 0> - static void execute(Class &cl, const Extra&... extra) { - using Base = typename Class::type; - /// Function which calls a specific C++ in-place constructor - cl.def("__init__", [](Base *self_, Args... args) { new (self_) Base(args...); }, extra...); - } - - template ::value, int> = 0> - static void execute(Class &cl, const Extra&... extra) { - using Base = typename Class::type; - using Alias = typename Class::type_alias; - handle cl_type = cl; - cl.def("__init__", [cl_type](handle self_, Args... args) { - if (self_.get_type().is(cl_type)) - new (self_.cast()) Base(args...); - else - new (self_.cast()) Alias(args...); - }, extra...); - } - - template ::value, int> = 0> - static void execute(Class &cl, const Extra&... extra) { - init_alias::execute(cl, extra...); - } -}; -template struct init_alias { - template ::value, int> = 0> - static void execute(Class &cl, const Extra&... extra) { - using Alias = typename Class::type_alias; - cl.def("__init__", [](Alias *self_, Args... args) { new (self_) Alias(args...); }, extra...); - } -}; inline void keep_alive_impl(handle nurse, handle patient) { diff --git a/tests/test_class.py b/tests/test_class.py index 7381c4ad8..9c5049f72 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -141,11 +141,8 @@ def test_operator_new_delete(capture): d = m.HasOpNewDelBoth() assert capture == """ A new 8 - A placement-new 8 B new 4 - B placement-new 4 D new 32 - D placement-new 32 """ sz_alias = str(m.AliasedHasOpNewDelSize.size_alias) sz_noalias = str(m.AliasedHasOpNewDelSize.size_noalias) @@ -153,8 +150,8 @@ def test_operator_new_delete(capture): c = m.AliasedHasOpNewDelSize() c2 = SubAliased() assert capture == ( - "C new " + sz_alias + "\nC placement-new " + sz_noalias + "\n" + - "C new " + sz_alias + "\nC placement-new " + sz_alias + "\n" + "C new " + sz_noalias + "\n" + + "C new " + sz_alias + "\n" ) with capture: diff --git a/tests/test_factory_constructors.cpp b/tests/test_factory_constructors.cpp index 54792d92c..767fe0edf 100644 --- a/tests/test_factory_constructors.cpp +++ b/tests/test_factory_constructors.cpp @@ -274,8 +274,9 @@ TEST_SUBMODULE(factory_constructors, m) { } int i; }; - // Workaround for a `py::init` on a class without placement new support + // As of 2.2, `py::init` no longer requires placement new py::class_(m, "NoPlacementNew") + .def(py::init()) .def(py::init([]() { return new NoPlacementNew(100); })) .def_readwrite("i", &NoPlacementNew::i) ; diff --git a/tests/test_factory_constructors.py b/tests/test_factory_constructors.py index 1d0679c2e..360137bc4 100644 --- a/tests/test_factory_constructors.py +++ b/tests/test_factory_constructors.py @@ -284,7 +284,19 @@ def test_init_factory_dual(): def test_no_placement_new(capture): - """Tests a workaround for `py::init<...>` with a class that doesn't support placement new.""" + """Prior to 2.2, `py::init<...>` relied on the type supporting placement + new; this tests a class without placement new support.""" + with capture: + a = m.NoPlacementNew(123) + + found = re.search(r'^operator new called, returning (\d+)\n$', str(capture)) + assert found + assert a.i == 123 + with capture: + del a + pytest.gc_collect() + assert capture == "operator delete called on " + found.group(1) + with capture: b = m.NoPlacementNew() diff --git a/tests/test_multiple_inheritance.py b/tests/test_multiple_inheritance.py index 80b6daeff..475dd3b3d 100644 --- a/tests/test_multiple_inheritance.py +++ b/tests/test_multiple_inheritance.py @@ -73,7 +73,8 @@ def test_multiple_inheritance_python(): class MI4(MI3, m.Base2): def __init__(self, i, j): MI3.__init__(self, i, j) - # m.Base2 is already initialized (via MI2) + # This should be ignored (Base2 is already initialized via MI2): + m.Base2.__init__(self, i + 100) class MI5(m.Base2, B1, m.Base1): def __init__(self, i, j): diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index 80ebe4ce2..953b390b8 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -148,6 +148,7 @@ class NCVirtTrampoline : public NCVirt { struct Base { /* for some reason MSVC2015 can't compile this if the function is pure virtual */ virtual std::string dispatch() const { return {}; }; + virtual ~Base() = default; }; struct DispatchIssue : Base { @@ -259,6 +260,7 @@ TEST_SUBMODULE(virtual_functions, m) { virtual std::string &str_ref() { return v; } virtual A A_value() { return a; } virtual A &A_ref() { return a; } + virtual ~OverrideTest() = default; }; class PyOverrideTest : public OverrideTest { @@ -311,6 +313,7 @@ public: \ return say_something(1) + " " + std::to_string(unlucky_number()); \ } A_METHODS + virtual ~A_Repeat() = default; }; class B_Repeat : public A_Repeat { #define B_METHODS \ @@ -335,7 +338,7 @@ D_METHODS }; // Base classes for templated inheritance trampolines. Identical to the repeat-everything version: -class A_Tpl { A_METHODS }; +class A_Tpl { A_METHODS; virtual ~A_Tpl() = default; }; class B_Tpl : public A_Tpl { B_METHODS }; class C_Tpl : public B_Tpl { C_METHODS }; class D_Tpl : public C_Tpl { D_METHODS };