Add Cpp::Box, the typed result carrier for the AOT/JIT boundary#1023
Merged
Conversation
vgvassilev
force-pushed
the
cpp-value-evaluate
branch
from
b77b6d4 to
7ec9635
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1023 +/- ##
==========================================
+ Coverage 86.17% 86.30% +0.12%
==========================================
Files 17 18 +1
Lines 5121 5227 +106
==========================================
+ Hits 4413 4511 +98
- Misses 708 716 +8
🚀 New features to boost your workflow:
|
vgvassilev
force-pushed
the
cpp-value-evaluate
branch
4 times, most recently
from
f3e4d98 to
18e25c8
Compare
Vipul-Cariappa
approved these changes
Jun 4, 2026
Collaborator
Vipul-Cariappa
left a comment
Vipul-Cariappa
left a comment
There was a problem hiding this comment.
Looks good to me! Modulo minor comments!
| #undef X | ||
| struct { | ||
| void* m_Ptr; | ||
| const ObjectOps* m_Ops; |
Collaborator
There was a problem hiding this comment.
Suggested change
| const ObjectOps* m_Ops; | |
| const ObjectOps m_Ops; |
Is storing the struct itself possible?
Comment on lines
+104
to
+106
| Cpp::Box sV = Cpp::Evaluate("struct S{} s; s"); | ||
| EXPECT_EQ(sV.getKind(), Cpp::Box::K_PtrOrObj); | ||
| EXPECT_NE(sV.getObjectPtr(), nullptr); |
Collaborator
There was a problem hiding this comment.
Can we also test the QualType stored? Using getType. I could not see getType being tested.
Collaborator
|
I guess the cppyy failure is expected. We will need to update all usages of |
Add a small tagged-union type, Cpp::Box, that bindings and the
interpreter use to exchange typed values in both directions. A binding
constructs one with Box::Create<T>(x) and hands it to the runtime; the
interpreter returns one from Evaluate and the binding extracts it with
unbox<T>(), convertTo<T>(), or visit(v). The same shape works for the
fundamental scalar types (stored inline) and for object payloads
(stored behind a refcounted pointer the producer attaches via a
{retain, release} ops table). Without Box every binding has to write
its own Kind switch and duplicate the per-type storage logic.
Evaluate gains a Box-returning C++ overload as the new primary form;
the body classifies the result by clang::QualType so the same code
serves both the clang-repl and cling backends. The legacy
intptr_t Evaluate(const char*, bool*) overload is preserved -- moved
into CXCppInterOp.cpp next to the other hand-written C-ABI bridges
along with its cppinterop_Evaluate C wrapper -- so existing C
consumers (cppyy, ROOT) keep working unchanged. Object payloads round-
trip through a small ValueRefCount wrapper in Compatibility.h next to
compat::Value; the wrapper uses clang::Value's copy ctor instead of
move ctor to work around an upstream bug fixed by
llvm/llvm-project#200888, and a static_assert(LLVM_VERSION_MAJOR < 23)
makes sure we revisit it.
Box.h itself is kept cheap to parse since the JIT pulls it whenever a
catalog thunk or evaluated snippet references Cpp::Box. We avoid
CppInterOpTypes.h (it transitively brings in <vector>, <string>,
<set>), use unsigned char for the Kind underlying type instead of
<cstdint>, gate <cassert> behind NDEBUG, and inline the cross-platform
always_inline and unreachable macros locally. Under NDEBUG the header
pulls in zero project or standard headers; with asserts on, only
<cassert>. Tests cover the fundamentals and K_PtrOrObj round-trip
(POD, non-trivial destructor, class-with-static-member), refcounted
copy + move semantics, and visit / convertTo on both AOT and runtime
paths.
vgvassilev
force-pushed
the
cpp-value-evaluate
branch
from
18e25c8 to
96dc713
Compare
Contributor
Author
|
osx26-x86-clang-cling-llvm20-cppyy seems stuck -- moving forward, the fixup pr is here compiler-research/cppyy#222 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a small tagged-union type, Cpp::Box, that bindings and the interpreter use to exchange typed values in both directions. A binding constructs one with Box::Create(x) and hands it to the runtime; the interpreter returns one from Evaluate and the binding extracts it with unbox(), convertTo(), or visit(v). The same shape works for the fundamental scalar types (stored inline) and for object payloads (stored behind a refcounted pointer the producer attaches via a {retain, release} ops table). Without Box every binding has to write its own Kind switch and duplicate the per-type storage logic.
Evaluate now returns a Box (replacing the old intptr_t + bool* out-param). Object payloads round-trip through a small ValueRefCount wrapper in Compatibility.h next to compat::Value; the wrapper uses clang::Value's copy ctor instead of move ctor to work around an upstream bug fixed by llvm/llvm-project#200888, and a static_assert(LLVM_VERSION_MAJOR < 23) makes sure we revisit it.
Box.h itself is kept cheap to parse since the JIT pulls it whenever a catalog thunk or evaluated snippet references Cpp::Box. We avoid CppInterOpTypes.h (it transitively brings in , , ), use unsigned char for the Kind underlying type instead of , gate behind NDEBUG, and inline the cross-platform always_inline macro locally. Under NDEBUG the header pulls in zero project or standard headers; with asserts on, only . Tests cover the fundamentals and K_PtrOrObj round-trip (POD, non-trivial destructor, class-with-static-member), refcounted copy + move semantics, and visit / convertTo on both AOT and runtime paths.