-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[core] Move TClass
tests to core/meta
#19586
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks.
Having twice the same symbols is indeed bad. What was the symptoms in the failures? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. (and indeed a test using TClass
belongs to core/meta).
Test Results 20 files 20 suites 3d 4h 24m 20s ⏱️ Results for commit 44e5bb2. ♻️ This comment has been updated with latest results. |
There were two symptoms with LLVM 20, depending on your "luck":
We focused on the second point and eventually I guessed what is going on when seeing the size of |
The test in core/clingutils is meant for unit testing of TClingUtils and statically links ClingUtils and Cling/Clang/LLVM libraries. Calling Cling, TInterpreter, or TClass results in two copies of the functions being around (in libCling.so and the test binary) with not much guarantees which ones are called at which moment.
Fix a couple of missing includes.
This is (unfortunately) needed for core/foundation headers.
The test in
core/clingutils
is meant for unit testing ofTClingUtils
and statically linksClingUtils
and Cling/Clang/LLVM libraries. Calling Cling,TInterpreter
, orTClass
results in two copies of the functions being around (inlibCling.so
and the test binary) with not much guarantees which ones are called at which moment.(causes a test failure with the upgrade to LLVM 20)