Skip to content

Commit 57b9a0a

Browse files
committed
function_record code health enhancements (std::launder, std::is_standard_layout)
https://chatgpt.com/share/67ea07fa-9afc-8008-95af-4d7e0829a9c2 Apply std::launder consistently to functional capture storage This commit improves the robustness and clarity of pybind11's internal capture mechanism for bound C++ callables stored in function_record. - Introduces `capture::from_data(void**)` to encapsulate reinterpretation of the in-place `capture` object stored in `rec->data`, ensuring consistent use of `std::launder`. - Adds a `static_assert` that verifies `capture` is standard layout, making explicit the precondition necessary for these low-level performance optimizations. - Centralizes the `PYBIND11_STD_LAUNDER` and `PYBIND11_HAS_STD_LAUNDER` macros into `common.h` to enable reuse across headers. - Replaces ad hoc laundering and strict aliasing warnings with a more principled and maintainable approach. This refines and generalizes the use of `std::launder` introduced in PR #2816 (2021-07-15), addressing undefined behavior more thoroughly while preserving ABI and performance characteristics.
1 parent a34fcdc commit 57b9a0a

File tree

4 files changed

+30
-11
lines changed

4 files changed

+30
-11
lines changed

include/pybind11/attr.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,12 @@ struct function_record {
272272
/// Pointer to next overload
273273
function_record *next = nullptr;
274274
};
275+
// The main purpose of this macro is to make it easy to pin-point the critically related code
276+
// sections.
277+
#define PYBIND11_ENSURE_PRECONDITION_FOR_FUNCTIONAL_H_PERFORMANCE_OPTIMIZATIONS(...) \
278+
static_assert( \
279+
__VA_ARGS__, \
280+
"Violation of precondition for pybind11/functional.h performance optimizations!")
275281

276282
/// Special data structure which (temporarily) holds metadata about a bound class
277283
struct type_record {

include/pybind11/detail/common.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,14 @@
123123
# endif
124124
#endif
125125

126+
#if defined(__cpp_lib_launder) && !(defined(_MSC_VER) && (_MSC_VER < 1914))
127+
# define PYBIND11_STD_LAUNDER std::launder
128+
# define PYBIND11_HAS_STD_LAUNDER 1
129+
#else
130+
# define PYBIND11_STD_LAUNDER
131+
# define PYBIND11_HAS_STD_LAUNDER 0
132+
#endif
133+
126134
#if defined(PYBIND11_CPP20)
127135
# define PYBIND11_CONSTINIT constinit
128136
# define PYBIND11_DTOR_CONSTEXPR constexpr

include/pybind11/functional.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,14 @@ struct type_caster<std::function<Return(Args...)>> {
101101
*reinterpret_cast<const std::type_info *>(rec->data[1]))) {
102102
struct capture {
103103
function_type f;
104+
105+
static capture *from_data(void **data) {
106+
return PYBIND11_STD_LAUNDER(reinterpret_cast<capture *>(data));
107+
}
104108
};
105-
value = ((capture *) &rec->data)->f;
109+
PYBIND11_ENSURE_PRECONDITION_FOR_FUNCTIONAL_H_PERFORMANCE_OPTIMIZATIONS(
110+
std::is_standard_layout<capture>::value);
111+
value = capture::from_data(rec->data)->f;
106112
return true;
107113
}
108114
rec = rec->next;

include/pybind11/pybind11.h

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,6 @@
3838
PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")
3939
#endif
4040

41-
#if defined(__cpp_lib_launder) && !(defined(_MSC_VER) && (_MSC_VER < 1914))
42-
# define PYBIND11_STD_LAUNDER std::launder
43-
# define PYBIND11_HAS_STD_LAUNDER 1
44-
#else
45-
# define PYBIND11_STD_LAUNDER
46-
# define PYBIND11_HAS_STD_LAUNDER 0
47-
#endif
4841
#if defined(__GNUG__) && !defined(__clang__)
4942
# include <cxxabi.h>
5043
#endif
@@ -345,6 +338,10 @@ class cpp_function : public function {
345338
using namespace detail;
346339
struct capture {
347340
remove_reference_t<Func> f;
341+
342+
static capture *from_data(void **data) {
343+
return PYBIND11_STD_LAUNDER(reinterpret_cast<capture *>(data));
344+
}
348345
};
349346

350347
/* Store the function including any extra state it might have (e.g. a lambda capture
@@ -364,7 +361,7 @@ class cpp_function : public function {
364361
PYBIND11_WARNING_DISABLE_GCC("-Wplacement-new")
365362
#endif
366363

367-
new ((capture *) &rec->data) capture{std::forward<Func>(f)};
364+
new (capture::from_data(rec->data)) capture{std::forward<Func>(f)};
368365

369366
#if !PYBIND11_HAS_STD_LAUNDER
370367
PYBIND11_WARNING_DISABLE_GCC("-Wstrict-aliasing")
@@ -374,8 +371,8 @@ class cpp_function : public function {
374371
// a significant refactoring it's "impossible" to solve.
375372
if (!std::is_trivially_destructible<capture>::value) {
376373
rec->free_data = [](function_record *r) {
377-
auto data = PYBIND11_STD_LAUNDER((capture *) &r->data);
378-
(void) data;
374+
auto data = capture::from_data(r->data);
375+
(void) data; // suppress "unused variable" warnings
379376
data->~capture();
380377
};
381378
}
@@ -492,6 +489,8 @@ class cpp_function : public function {
492489
using FunctionType = Return (*)(Args...);
493490
constexpr bool is_function_ptr
494491
= std::is_convertible<Func, FunctionType>::value && sizeof(capture) == sizeof(void *);
492+
PYBIND11_ENSURE_PRECONDITION_FOR_FUNCTIONAL_H_PERFORMANCE_OPTIMIZATIONS(
493+
!is_function_ptr || std::is_standard_layout<capture>::value);
495494
if (is_function_ptr) {
496495
rec->is_stateless = true;
497496
rec->data[1]

0 commit comments

Comments
 (0)