Repository navigation
[cppyy] Avoid JIT-compiling make_unique/make_shared wrappers for builtins - #20977
Conversation
8ce3445 to
116b21c
Compare
int and float in make_unique optimizationint as possible for make_unique optimization
116b21c to
489a32b
Compare
int as possible for make_unique optimizationint and float for make_unique optimization
25de749 to
7a5a2b9
Compare
Test Results 23 files 23 suites 3d 18h 48m 7s ⏱️ For more details on these failures, see this check. Results for commit bdab6cc. ♻️ This comment has been updated with latest results. |
84d4744 to
211c619
Compare
int and float for make_unique optimization…tins
std::make_unique/make_shared for a fundamental element type forces the
interpreter to JIT a call wrapper whose variadic Args... template pack is
mis-desugared on some platforms (macOS with C++ modules) into invalid code
such as `make_unique<int, 0>`, breaking e.g. make_unique<int>.
Route arithmetic element types through dedicated, non-variadic helpers
(__cppyy_internal::make_unique_scalar / make_shared_scalar) that build the
smart pointer from a heap-allocated scalar (new T{} / new T(v)), side-stepping
the make_unique/make_shared wrapper entirely. The helpers static_assert that
the element type is arithmetic, mirroring the Python-side dispatch. Class types
keep the existing pure-introspection adopt path, which never instantiated
make_unique either.
Arithmetic element types are detected by querying std::is_arithmetic through
the interpreter, letting C++ be the source of truth: canonical spellings and
typedefs (size_t, Int_t, ...) resolve correctly instead of silently yielding a
null smart pointer. is_arithmetic (rather than is_fundamental) also excludes
void and std::nullptr_t, which cannot back a heap scalar.
This also fixes make_unique/make_shared for fundamental spellings other than
int/float (e.g. 'double', 'long'), which previously errored.
Re-enables the cppyy test15/test16 unit tests that were xfailed on macOS/Windows.
Closes root-project#19122
|
@vepadulano Thanks for the excellent suggestions! I have rewritten the logic around |
|
Failures like the one of |
vepadulano
left a comment
There was a problem hiding this comment.
Clever solution given the constraints! Ideally I would have preferred a C++ only solution for all types using if constexpr to differentiate between arithmetic and not. But I see the non-arithmetic types need Python-side treatment anyway, so it's probably out of scope for this PR. Thanks for the effort!
std::make_unique/make_shared for a fundamental element type forces the interpreter to JIT a call wrapper whose variadic Args... template pack is mis-desugared on some platforms (macOS with C++ modules) into invalid code such as
make_unique<int, 0>, breaking e.g. make_unique.Route arithmetic element types through dedicated, non-variadic helpers (__cppyy_internal::make_unique_scalar / make_shared_scalar) that build the smart pointer from a heap-allocated scalar (new T{} / new T(v)), side-stepping the make_unique/make_shared wrapper entirely. The helpers static_assert that the element type is arithmetic, mirroring the Python-side dispatch. Class types keep the existing pure-introspection adopt path, which never instantiated make_unique either.
Arithmetic element types are detected by querying std::is_arithmetic through the interpreter, letting C++ be the source of truth: canonical spellings and typedefs (size_t, Int_t, ...) resolve correctly instead of silently yielding a null smart pointer. is_arithmetic (rather than is_fundamental) also excludes void and std::nullptr_t, which cannot back a heap scalar.
This also fixes make_unique/make_shared for fundamental spellings other than int/float (e.g. 'double', 'long'), which previously errored.
Re-enables the cppyy test15/test16 unit tests that were xfailed on macOS/Windows.
Closes #19122