-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[cling] Unload all redecls of ClassTemplateDecl #19394
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
base: master
Are you sure you want to change the base?
Conversation
When unloading a ClassTemplateDecl, no redeclarations were handled Additional redecls may still be part of the chain leading to duplicates and broken redecl chains that cause assertion failures like: "Passed first decl twice, invalid redecl chain!" Fixes assertion failures post llvm commit: llvm/llvm-project@17d8ed7 for the test `roottest-root-meta-cmsUnload-make`
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 for a test case?
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. In my understanding, this currently leaves the AST in a partially invalid state. I'm not sure if this is easy to test at the moment, but will be exercised by the upgrade.
I've been looking into adding a test case, but it's a bit difficult to test this within the ROOT/Cling testing scope. We could add a simplified test similar to |
Ok, any chance in remembering to add the test when that lands. I presume the test case is unloading the test case introduced by the commit you mentioned. |
The test with full commit for reference: 08e304b I was using a simplified version of this test for debugging. But the above test should cover the case when unloading after the upgrade.
|
So just to summarize: |
|
When unloading a ClassTemplateDecl, no redeclarations were handled.
Additional redecls may still be part of the chain leading to duplicates
and broken redecl chains that cause assertion failures like:
"Passed first decl twice, invalid redecl chain!"
Fixes assertion failures post llvm commit:
llvm/llvm-project@17d8ed7
for the test
roottest-root-meta-cmsUnload-make
This Pull request:
Changes or fixes:
Checklist:
This PR fixes #
The failing test
roottest-root-meta-cmsUnload-make
in #19242