-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[TCling] Do not add decls for control statements if already annotated #15368
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
Test Results 17 files 17 suites 3d 13h 21m 43s ⏱️ For more details on these failures, see this check. Results for commit 1c45df8. ♻️ This comment has been updated with latest results. |
978cafb
to
1b3bab6
Compare
Not a proper solution. What we ideally want to do is check if a |
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.
Any chance in adding a test?
I also think a test would be good. @vgvassilev in which subdirectory would you put the simple macro which was crashing in https://github.com/root-project/roottest/tree/master/cling ? |
Maybe we can do a death test in the test folder of core/metacling. |
9b0507a
to
11ba471
Compare
rebased & test added |
11ba471
to
c97b32c
Compare
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 a lot for this fix. Once the tests are green, afaik we are ready to merge.
c97b32c
to
04339e5
Compare
I changed the test. Before we were also testing the "line" number according to the interpreter, which can be platform dependent. Now we are independent from that |
Can it be that this fix fixes also #15537 ? That's what I see, I think... |
As a note, once we remove |
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!
Still fails for me. It seems important to put |
04339e5
to
ce44a75
Compare
ce44a75
to
0d357e3
Compare
0d357e3
to
1c45df8
Compare
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!
This Pull request:
Changes or fixes:
Checklist:
This PR fixes #8367