[cling] Unload all redecls of ClassTemplateDecl

[devajithvs]

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:

• tested changes locally
• updated the docs (if necessary)

This PR fixes #
The failing test roottest-root-meta-cmsUnload-make in #19242

[vgvassilev]

Any chance for a test case?

[hahnjo]

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.

[devajithvs]

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 roottest-root-meta-cmsUnload-make, but will need LLVM commit llvm/llvm-project@17d8ed7 to trigger an assertion.

[vgvassilev]

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.

[devajithvs]

Ok, any chance in remembering to add the test when that lands.

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.

case1.script

#include "inc/Nothing.h"
// gDebug=1;
.L lib/libEdm.so
.L lib/libStrip.so
// step2.cxx does a lookup that provokes a auto-parsing following by
// an (intentional) compilation error and thus a set of unloading.
.x scripts/step2.cxx
// This use to lead to a strange error (complain that a class is a being
// defined a second time on the ... same line in the same file) which was
// due to the unloading now removing the DefinitionData cached in the forward
// declaration decls left (intentionally) by the unloading.
gInterpreter->AutoParse("reco::utils::ClusterTotals");

[hahnjo]

So just to summarize: cmsUnload will test this fix automatically after the upgrade. I don't see a need to add another test for this.

[vgvassilev]

cmsUnload is a very complex test which is at integration level, I wanted something that tests the particular feature and when it regresses we can debug and fix it easily without having to build ROOT.

[devajithvs]

Added a simplified test!

[github-actions]

Test Results

    21 files      21 suites   3d 13h 12m 34s ⏱️
 3 681 tests  3 681 ✅ 0 💤 0 ❌
75 451 runs  75 451 ✅ 0 💤 0 ❌

Results for commit 0bb874b.

♻️ This comment has been updated with latest results.

[vgvassilev]

I was imagining a cling test but that also should do.