=?utf-8?q?Balázs_Kéri?= <balazs.k...@ericsson.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/101...@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);
----------------
danix800 wrote:
I digged a little bit about template importing and have a few questions:
1. Templates are redeclarable, but not like some other nodes (`FunctionDecl`/
`VarDecl`/`TagDecl` currently), no redecls are imported ahead of the
current one.
if redecls are imported, the ordering problem might go away. If this works,
we could apply the same mechnism to some other redeclarables too (I tested
locally and templates can be imported with correct order).
Or, there might be other considerations about not importing redecls for some
nodes (like performance? but redecls are usually small, right?), for some
client
(StaticAnalyzer), this mis-ordering is still acceptable, but I'm not quite
sure how
many clients depend on the importer and how they depends on the ordering.
2. For template parameters, current implementation supports importing them
directly
but added it into the top-level TU context, which might not be the good for
any
clients, not to say about ODR violations. I'm not clear what's the
consideration
of not importing some of the nodes without importing context firstly.
For the crash fix it's a LGTM.
https://github.com/llvm/llvm-project/pull/101836
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
