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

[Matheus Izvekov via cfe-commits]

=?utf-8?q?Balázs_Kéri?= <balazs.keri at ericsson.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/101836 at github.com>


================
@@ -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);
----------------
mizvekov wrote:

Yes I understand the redeclaration chain order is not reliable with AST import. It however has no semantic implications.

But making the default argument inheritance chain based on the redeclaration chain order is not helpful, as it will make the AST represent seemingly nonsensical things, like a certain use of a declaration without a default argument succeed in a use which would need it.

The following properties are important to be preserved:
* Declarations which did not have a default argument definition visible should keep so after import.
* The declaration which introduced the default argument definition should keep being so after import.
* Declarations which had the default argument inherited should keep being so after import.

If inherited default arguments always point to the argument with the definition, instead of a previously inherited entry, which I think should already be the case, then this won't create many difficulties here.

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