Skip to content

Commit cd5fbc5

Browse files
committed
Replacing virtual guarded_operator_call with non-virtual guarded_delete.
1 parent 1e6cc9d commit cd5fbc5

File tree

1 file changed

+23
-49
lines changed

1 file changed

+23
-49
lines changed

include/pybind11/detail/smart_holder_poc.h

+23-49
Original file line numberDiff line numberDiff line change
@@ -56,33 +56,14 @@ High-level aspects:
5656
namespace pybindit {
5757
namespace memory {
5858

59-
struct guarded_operator_call {
59+
struct guarded_delete {
60+
void (*del_ptr)(void *);
6061
bool armed_flag;
61-
explicit guarded_operator_call(bool armed_flag) : armed_flag{armed_flag} {}
62-
virtual void operator()(void *) = 0;
63-
virtual ~guarded_operator_call() = default;
64-
65-
// Some compilers complain if the implicitly defined copy constructor is used.
66-
guarded_operator_call(const guarded_operator_call &) = default;
67-
};
68-
69-
template <typename T>
70-
struct guarded_builtin_delete : guarded_operator_call {
71-
explicit guarded_builtin_delete(bool armed_flag) : guarded_operator_call{armed_flag} {}
72-
void operator()(void *raw_ptr) override { delete_impl<T>(raw_ptr); }
73-
template <typename T_ = T,
74-
typename std::enable_if<std::is_destructible<T_>::value, int>::type = 0>
75-
void delete_impl(void *raw_ptr) {
62+
guarded_delete(void (*del_ptr)(void *), bool armed_flag)
63+
: del_ptr{del_ptr}, armed_flag{armed_flag} {}
64+
void operator()(void *raw_ptr) const {
7665
if (armed_flag)
77-
delete (T *) raw_ptr;
78-
}
79-
template <typename T_ = T,
80-
typename std::enable_if<!std::is_destructible<T_>::value, int>::type = 0>
81-
void delete_impl(void *) {
82-
// This noop operator is needed to avoid a compilation error (for `delete raw_ptr;`), but
83-
// throwing an exception from here could std::terminate the process. Therefore the runtime
84-
// check for lifetime-management correctness is implemented elsewhere (in
85-
// ensure_pointee_is_destructible()).
66+
(*del_ptr)(raw_ptr);
8667
}
8768
};
8869

@@ -105,12 +86,13 @@ guarded_delete make_guarded_builtin_delete(bool armed_flag) {
10586
}
10687

10788
template <typename T, typename D>
108-
struct guarded_custom_deleter : guarded_operator_call {
109-
explicit guarded_custom_deleter(bool armed_flag) : guarded_operator_call{armed_flag} {}
110-
void operator()(void *raw_ptr) override {
111-
if (armed_flag)
112-
D()((T *) raw_ptr);
113-
}
89+
inline void custom_delete(void *raw_ptr) {
90+
D()((T *) raw_ptr);
91+
}
92+
93+
template <typename T, typename D>
94+
guarded_delete make_guarded_custom_deleter(bool armed_flag) {
95+
return guarded_delete(custom_delete<T, D>, armed_flag);
11496
};
11597

11698
template <typename T>
@@ -121,7 +103,6 @@ inline bool is_std_default_delete(const std::type_info &rtti_deleter) {
121103

122104
struct smart_holder {
123105
const std::type_info *rtti_uqp_del = nullptr;
124-
guarded_operator_call *vptr_del = nullptr;
125106
std::shared_ptr<void> vptr;
126107
bool vptr_is_using_noop_deleter : 1;
127108
bool vptr_is_using_builtin_delete : 1;
@@ -213,25 +194,20 @@ struct smart_holder {
213194
}
214195
}
215196

216-
template <typename T, typename D>
217-
void vptr_reset(T *raw_ptr, const D &del) {
218-
vptr.reset(raw_ptr, del); // Copies del.
219-
// The const_cast is only for certain compilers (Ubuntu 20 GCC 6.3.0 being one).
220-
vptr_del = const_cast<D *>(std::get_deleter<D>(vptr)); // Pointer to copy of del.
221-
}
222-
223197
void reset_vptr_deleter_armed_flag(bool armed_flag) const {
224-
if (vptr_del == nullptr) {
198+
// The const_cast is only for certain compilers (Ubuntu 20 GCC 6.3.0 being one).
199+
auto vptr_del_ptr = const_cast<guarded_delete *>(std::get_deleter<guarded_delete>(vptr));
200+
if (vptr_del_ptr == nullptr) {
225201
throw std::runtime_error(
226202
"smart_holder::reset_vptr_deleter_armed_flag() called in an invalid context.");
227203
}
228-
vptr_del->armed_flag = armed_flag;
204+
vptr_del_ptr->armed_flag = armed_flag;
229205
}
230206

231207
template <typename T>
232208
static smart_holder from_raw_ptr_unowned(T *raw_ptr) {
233209
smart_holder hld;
234-
hld.vptr_reset(raw_ptr, guarded_builtin_delete<T>(false));
210+
hld.vptr.reset(raw_ptr, [](void *) {});
235211
hld.vptr_is_using_noop_deleter = true;
236212
hld.is_populated = true;
237213
return hld;
@@ -262,7 +238,7 @@ struct smart_holder {
262238
static smart_holder from_raw_ptr_take_ownership(T *raw_ptr) {
263239
ensure_pointee_is_destructible<T>("from_raw_ptr_take_ownership");
264240
smart_holder hld;
265-
hld.vptr_reset(raw_ptr, guarded_builtin_delete<T>(true));
241+
hld.vptr.reset(raw_ptr, make_guarded_builtin_delete<T>(true));
266242
hld.vptr_is_using_builtin_delete = true;
267243
hld.is_populated = true;
268244
return hld;
@@ -281,10 +257,7 @@ struct smart_holder {
281257
}
282258

283259
// Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details).
284-
void release_disowned() {
285-
vptr.reset();
286-
vptr_del = nullptr;
287-
}
260+
void release_disowned() { vptr.reset(); }
288261

289262
// SMART_HOLDER_WIP: review this function.
290263
void ensure_can_release_ownership(const char *context = "ensure_can_release_ownership") const {
@@ -313,9 +286,10 @@ struct smart_holder {
313286
hld.rtti_uqp_del = &typeid(D);
314287
hld.vptr_is_using_builtin_delete = is_std_default_delete<T>(*hld.rtti_uqp_del);
315288
if (hld.vptr_is_using_builtin_delete) {
316-
hld.vptr_reset(unq_ptr.get(), guarded_builtin_delete<T>(true));
289+
hld.vptr.reset(unq_ptr.get(), make_guarded_builtin_delete<T>(true));
317290
} else {
318-
hld.vptr_reset(unq_ptr.get(), guarded_custom_deleter<T, D>(true));
291+
make_guarded_custom_deleter<T, D>(false);
292+
hld.vptr.reset(unq_ptr.get(), make_guarded_custom_deleter<T, D>(true));
319293
}
320294
unq_ptr.release();
321295
hld.is_populated = true;

0 commit comments

Comments
 (0)