[clang] [clang][ASTImporter] New fix for default template parameter values. (PR #101836)

[Balázs Kéri via cfe-commits]

================
@@ -5968,11 +5962,21 @@ ASTNodeImporter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
   }
 
   if (D->hasDefaultArgument()) {
+    // Default argument can be "inherited" when it has a reference to the
+    // previous declaration (of the default argument) which is stored only once.
+    // Here we import the default argument in any case, and the inherited state
+    // is updated later after the parent template was created. If the
+    // inherited-from object would be imported here it causes more difficulties
+    // (parent template may not be created yet and import loops can occur).
     Expected<TemplateArgumentLoc> ToDefaultArgOrErr =
         import(D->getDefaultArgument());
     if (!ToDefaultArgOrErr)
       return ToDefaultArgOrErr.takeError();
-    ToD->setDefaultArgument(ToD->getASTContext(), *ToDefaultArgOrErr);
+    // The just called import process can trigger import of the parent template
+    // which can update the default argument value to "inherited". This should
+    // not be changed.
+    if (!ToD->hasDefaultArgument())
+      ToD->setDefaultArgument(ToD->getASTContext(), *ToDefaultArgOrErr);
----------------
balazske wrote:

I made a new fix where the original inheritance order does not change. There are new problems with this solution but probably it is acceptable.
If there is an existing declaration `template <int I = 1> struct X;` and the same code is imported, the declaration is duplicated because it is not a definition (this is how `ASTImporter` generally works). In this case the default value `I = 3` is imported too. The AST is already invalid after this import (default value can be specified only once). With a fix like the previous solution this problem does not exist.
Another problem is with existing code
```
template <int I = 1> struct X {};
```
and imported code
```
template <int I = 1> struct X {};
template <int I> struct X;
```
The second declaration is imported with inherited default argument but the first is not imported, only mapped to the existing one. The "inherited from" value in the second declaration (in the To AST) is not linked to the original one, a new object is created instead that is "invisible" from the AST (only reachable with the `getInheritedFrom` call).

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