[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

Shafik Yaghmour via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 28 16:49:57 PDT 2019

shafik added a comment.

I don't see any regressions but I am a little uncomfortable since there are no tests. So I would feel better if this was split into three parts: Namespaces, Enums and Templates.

Are there small test programs that fail due to the missing data? We can add them as regression tests.

Comment at: lib/AST/ASTImporter.cpp:6129
     if (Error Err =
-        ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+            ImportTemplateArgumentListInfo(E->getLAngleLoc(), E->getRAngleLoc(),
+                                           E->template_arguments(), ToTAInfo))
Curious why you decided to add the new arguments to the front as opposed to the end?

Comment at: lib/AST/ASTImporter.cpp:7150
+  auto Imp = importSeq(E->getQualifierLoc(), E->getTemplateKeywordLoc(),
+                       E->getDeclName(), E->getNameInfo().getLoc(),
+                       E->getLAngleLoc(), E->getRAngleLoc());
Can you explain why `E->getNameInfo().getLoc()` is more correct than `E->getExprLoc()`?

Comment at: lib/AST/ASTImporter.cpp:7225
-  if (E->hasExplicitTemplateArgs() && E->getTemplateKeywordLoc().isValid()) {
+  if (E->hasExplicitTemplateArgs()) {
     TemplateArgumentListInfo ToTAInfo;
We still want to import a few lines down even if `!E->getTemplateKeywordLoc().isValid()`?

  rC Clang



More information about the cfe-commits mailing list