Skip to content

Commit 3ba8721

Browse files
committed
Use py::init(...) for py::init<...>()
This simplifies the py::init<...> implementations slightly (especially the Base-vs-Alias one), while also no longer requiring that a class support placement new to be able to use `py::init<...>()`. This also exposes a few warnings (fixed here) from deleting a pointer to a base class with virtual functions but without a virtual destructor. These look like legitimate warnings that we shouldn't suppress; this adds virtual destructors to the appropriate classes.
1 parent 301ad17 commit 3ba8721

File tree

4 files changed

+19
-22
lines changed

4 files changed

+19
-22
lines changed

include/pybind11/pybind11.h

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,8 +1364,7 @@ template <typename... Args> struct init {
13641364
template <typename Class, typename... Extra, enable_if_t<!Class::has_alias, int> = 0>
13651365
static void execute(Class &cl, const Extra&... extra) {
13661366
using Base = typename Class::type;
1367-
/// Function which calls a specific C++ in-place constructor
1368-
cl.def("__init__", [](Base *self_, Args... args) { new (self_) Base(args...); }, extra...);
1367+
cl.def(pybind11::init([](Args... args) { return new Base(args...); }), extra...);
13691368
}
13701369

13711370
template <typename Class, typename... Extra,
@@ -1374,13 +1373,10 @@ template <typename... Args> struct init {
13741373
static void execute(Class &cl, const Extra&... extra) {
13751374
using Base = typename Class::type;
13761375
using Alias = typename Class::type_alias;
1377-
handle cl_type = cl;
1378-
cl.def("__init__", [cl_type](handle self_, Args... args) {
1379-
if (self_.get_type().is(cl_type))
1380-
new (self_.cast<Base *>()) Base(args...);
1381-
else
1382-
new (self_.cast<Alias *>()) Alias(args...);
1383-
}, extra...);
1376+
cl.def(pybind11::init(
1377+
[](Args... args) { return new Base(args...); },
1378+
[](Args... args) { return new Alias(args...); }),
1379+
extra...);
13841380
}
13851381

13861382
template <typename Class, typename... Extra,
@@ -1395,7 +1391,8 @@ template <typename... Args> struct init_alias {
13951391
enable_if_t<Class::has_alias && std::is_constructible<typename Class::type_alias, Args...>::value, int> = 0>
13961392
static void execute(Class &cl, const Extra&... extra) {
13971393
using Alias = typename Class::type_alias;
1398-
cl.def("__init__", [](Alias *self_, Args... args) { new (self_) Alias(args...); }, extra...);
1394+
cl.def(pybind11::init([](Args... args) { return new Alias(args...); }),
1395+
extra...);
13991396
}
14001397
};
14011398

tests/test_class.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,20 +141,17 @@ class SubAliased(m.AliasedHasOpNewDelSize):
141141
d = m.HasOpNewDelBoth()
142142
assert capture == """
143143
A new 8
144-
A placement-new 8
145144
B new 4
146-
B placement-new 4
147145
D new 32
148-
D placement-new 32
149146
"""
150147
sz_alias = str(m.AliasedHasOpNewDelSize.size_alias)
151148
sz_noalias = str(m.AliasedHasOpNewDelSize.size_noalias)
152149
with capture:
153150
c = m.AliasedHasOpNewDelSize()
154151
c2 = SubAliased()
155152
assert capture == (
156-
"C new " + sz_alias + "\nC placement-new " + sz_noalias + "\n" +
157-
"C new " + sz_alias + "\nC placement-new " + sz_alias + "\n"
153+
"C new " + sz_noalias + "\n" +
154+
"C new " + sz_alias + "\n"
158155
)
159156

160157
with capture:

tests/test_multiple_inheritance.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ def __init__(self, i, j):
7171
MI2.__init__(self, i, j)
7272

7373
class MI4(MI3, m.Base2):
74-
def __init__(self, i, j, k):
75-
MI3.__init__(self, j, k)
76-
m.Base2.__init__(self, i)
74+
def __init__(self, i, j):
75+
MI3.__init__(self, i, j)
76+
# m.Base2 already initialized via MI2
7777

7878
class MI5(m.Base2, B1, m.Base1):
7979
def __init__(self, i, j):
@@ -127,10 +127,10 @@ def __init__(self, i):
127127
assert mi3.foo() == 5
128128
assert mi3.bar() == 6
129129

130-
mi4 = MI4(7, 8, 9)
130+
mi4 = MI4(7, 8)
131131
assert mi4.v() == 1
132-
assert mi4.foo() == 8
133-
assert mi4.bar() == 7
132+
assert mi4.foo() == 7
133+
assert mi4.bar() == 8
134134

135135
mi5 = MI5(10, 11)
136136
assert mi5.v() == 1

tests/test_virtual_functions.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ public: \
179179
return say_something(1) + " " + std::to_string(unlucky_number()); \
180180
}
181181
A_METHODS
182+
virtual ~A_Repeat() = default;
182183
};
183184
class B_Repeat : public A_Repeat {
184185
#define B_METHODS \
@@ -203,7 +204,7 @@ D_METHODS
203204
};
204205

205206
// Base classes for templated inheritance trampolines. Identical to the repeat-everything version:
206-
class A_Tpl { A_METHODS };
207+
class A_Tpl { A_METHODS; virtual ~A_Tpl() = default; };
207208
class B_Tpl : public A_Tpl { B_METHODS };
208209
class C_Tpl : public B_Tpl { C_METHODS };
209210
class D_Tpl : public C_Tpl { D_METHODS };
@@ -314,6 +315,7 @@ void initialize_inherited_virtuals(py::module &m) {
314315
struct Base {
315316
/* for some reason MSVC2015 can't compile this if the function is pure virtual */
316317
virtual std::string dispatch() const { return {}; };
318+
virtual ~Base() = default;
317319
};
318320

319321
struct DispatchIssue : Base {
@@ -416,6 +418,7 @@ TEST_SUBMODULE(virtual_functions, m) {
416418
virtual std::string &str_ref() { return v; }
417419
virtual A A_value() { return a; }
418420
virtual A &A_ref() { return a; }
421+
virtual ~OverrideTest() = default;
419422
};
420423

421424
class PyOverrideTest : public OverrideTest {

0 commit comments

Comments
 (0)