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

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

balazske marked 3 inline comments as done.
balazske added inline comments.


================
Comment at: lib/AST/ASTImporter.cpp:6129
     if (Error Err =
-        ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+            ImportTemplateArgumentListInfo(E->getLAngleLoc(), E->getRAngleLoc(),
+                                           E->template_arguments(), ToTAInfo))
----------------
shafik wrote:
> Curious why you decided to add the new arguments to the front as opposed to the end?
This overload of `ImportTemplateArgumentListInfo` already exists before the patch. It looks like that the last argument is the output value and the arguments before are input values.


================
Comment at: lib/AST/ASTImporter.cpp:7150
+  auto Imp = importSeq(E->getQualifierLoc(), E->getTemplateKeywordLoc(),
+                       E->getDeclName(), E->getNameInfo().getLoc(),
+                       E->getLAngleLoc(), E->getRAngleLoc());
----------------
shafik wrote:
> Can you explain why `E->getNameInfo().getLoc()` is more correct than `E->getExprLoc()`?
The getExprLoc returns (if I am correct) the begin location and `getNameInfo().getLoc()` returns in "X<T>::value" location of "value" (this should not be the same as BeginLoc, may be it is the EndLoc?). (The begin and end location is not imported explicitly, it is obtained from other location values in the expression object.) I just observed that `E->getLocation()` can be used instead of `E->getNameInfo().getLoc()`.
(The reason why this is correct that the test in D60463 indicates failure if getExprLoc is used, the begin and end locations are the same. That test works only in our Ericsson fork because Decl reordering issue.)


================
Comment at: lib/AST/ASTImporter.cpp:7225
 
-  if (E->hasExplicitTemplateArgs() && E->getTemplateKeywordLoc().isValid()) {
+  if (E->hasExplicitTemplateArgs()) {
     TemplateArgumentListInfo ToTAInfo;
----------------
shafik wrote:
> We still want to import a few lines down even if `!E->getTemplateKeywordLoc().isValid()`?
The TemplateKeywordLoc can be invalid if the optional `template` keyword is missing. The import function can import an invalid SourceLocation (as invalid but not error).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60499