[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

================
@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
     return true;
 
+  // The external source may have additional definitions of this entity that are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+    Source->CompleteRedeclChain(D);
+  }
----------------
mpark wrote:

So the issue has to do with `ClassTemplateDecl`, `ClassTemplateSpecializationDecl`, and `CXXRecordDecl`.
I'll describe my observations in the problematic invocation of `hasAcceptableDefinition`.

I believe there are 4 relevant pieces here:

1. `ClassTemplateSpecializationDecl` for `BS<Empty<int>>`
2. `CXXRecordDecl` for `BS<Empty<int>>` (base portion of (1))
3. `ClassTemplateDecl` for `BS<T>`
4. `CXXRecordDecl` for `BS<T>`

`NamedDecl* D` coming into `hasAcceptableDefinition` points to a `NamedDecl` base portion of `ClassTemplateSpecializationDecl` for `BS<Empty<int>>` [1]. `auto* RD = dyn_cast<CXXRecordDecl>(D)` is the `CXXRecordDecl` portion of `BS<Empty<int>>` [2]. `auto *Pattern = RD->getTemplateInstantiationPattern()` is an instance of `CXXRecordDecl` for `BS<T>` [4].

Whatever the reason, at this point the *definition* of [4] has not been loaded yet. I would've expected that the call to `RD->getDefinition()` (which has a dummy invocation to `getMostRecentDecl()` under the hood) should load the definition, but this does not happen for some reason.

However, calling `getMostRecentDecl()` on any of [1], [2] or [3] have the desired effect of loading the definition of [4]... e.g. `RD->getMostRecentDecl()` calls it on [2] which loads the definition of [4], and `Pattern->getDescribedClassTemplate()->getMostRecentDecl()` calls it on [3] which also gives us the definition of [4] we're looking for...

The current solution here effectively is to call `getMostRecentDecl` on [2] such that it loads the redecl chain which somehow includes the definition of [4]... but maybe the real solution is to figure out why the `getMostRecentDecl` call on [4] itself doesn't load the corresponding definition?

https://github.com/llvm/llvm-project/pull/129982