[cppinterop] Upgrade to latest for cppyy migration - #22728
Conversation
Test Results 23 files 23 suites 3d 15h 25m 57s ⏱️ Results for commit 11a13bb. ♻️ This comment has been updated with latest results. |
|
@aaronj0, I believe we need to also move |
Yes, I have that obvious change and more patches in the pipeline.. (adapting to the new types, etc) |
7618eb1 to
8f4c938
Compare
7fa2272 to
3427288
Compare
09a28ec to
4dbc826
Compare
|
I believe this upgrade now passes the CI, the windows x64 failures are visible on master. I will upstream the required patches and bump this upgrade to the latest commit. cc @guitargeek |
4dbc826 to
a0a483b
Compare
guitargeek
left a comment
guitargeek
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the update. Unfortunately we don't have green builds on Windows, but that problem is also present in the nightlies.
83e29a3 to
523fd79
Compare
The latest CppInterOp upgrade introduces opaque-handle structs Cpp::ObjectRef, Cpp::DeclRef, and Cpp::InterpRef (CppInterOpTypes.h) to replace void*/typedef pointer arguments.
The latest CppInterOp upgrade changes UseExternalInterpreter to also install a CppInteropDiagConsumer onto Cling's DiagnosticsEngine. Rootcling later wraps the current diag client with CheckModuleBuildClient via DiagnosticsEngine::setClient which deletes CppInterOp's previously-owned client. This leads to CheckModuleBuildClient holding a dangling fChild pointer. Without this patch, cling::Interpreter::ShutDown crashes. Since rootcling does not use CppInterOp, we can avoid registering the interpreter with CppInterOp if we are running in rootcling.
523fd79 to
11a13bb
Compare
3 participants
Test and land the latest upgrade, required to rebase the cppyy migration PR on top