-
Notifications
You must be signed in to change notification settings - Fork 31
[interpreter] Drop IncrementalExecutor offsets to obtain LLJIT for Cling #589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@aaronj0 When I was looking at this in my PR to drop cling 1.0 support I was planning to replace every occurance of compat::getExecutionEngine in the code (given its now just a return statement). I just never got around to doing it. Maybe you can do it here? |
clang-tidy review says "All clean, LGTM! 👍" |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #589 +/- ##
=======================================
Coverage 76.43% 76.43%
=======================================
Files 9 9
Lines 3666 3666
=======================================
Hits 2802 2802
Misses 864 864
🚀 New features to boost your workflow:
|
I prefer to land this here as an atomic change...
hmm that doesn't make sense to me: In a function that accepts a CppInterOp/lib/Interpreter/CppInterOp.cpp Lines 3160 to 3162 in 392b754
That uses this API: CppInterOp/lib/Interpreter/CppInterOp.cpp Lines 3212 to 3217 in 392b754
we need the abstraction anyway since clang-repl requires extra handling: CppInterOp/lib/Interpreter/Compatibility.h Lines 342 to 345 in 392b754
Another motivation is also to keep the |
This patch drops pointer arithmetic used to obtain the
llvm::orc::LLJIT
from older Cling versions. This would make for safer usage of thecling::Interpreter
and good to have for the 1.7.0 stable release that ROOT will move to.