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

[Shafik Yaghmour via Phabricator via cfe-commits]

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()`?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60499/new/

https://reviews.llvm.org/D60499