[PATCH] D14326: ASTImporter: expressions, pt.2

[Serge Pavlov via cfe-commits]

sepavloff added a comment.

The patch requires tests.
For instance, import of CXXBaseSpecifier can be tested along with static_cast: if a class indeed is a base, static_cast will be processed silent, otherwise a message must appear. ArraySubscript could be tested using constexpr function. MaterializeTemporaryExpr, CXXBindTemporaryExpr and CXXBindTemporaryExpr probably also can be tested with constexpr functions if temporary object class has constexpr constructor.


================
Comment at: lib/AST/ASTImporter.cpp:5614
@@ +5613,3 @@
+  return new (Importer.getToContext()) ArrayTypeTraitExpr(
+        Importer.Import(E->getExprLoc()), E->getTrait(), ToQueried,
+        E->getValue(), Dim, Importer.Import(E->getLocEnd()), T);
----------------
Use `getStartLoc` instead of `getExprLoc`. It more clearly resolves to `ArrayTypeTraitExpr ::Loc`.

================
Comment at: lib/AST/ASTImporter.cpp:5628
@@ +5627,3 @@
+  return new (Importer.getToContext()) ExpressionTraitExpr(
+        Importer.Import(E->getExprLoc()), E->getTrait(), ToQueried,
+        E->getValue(), Importer.Import(E->getLocEnd()), T);
----------------
`getExprLoc` -> `getStartLoc`.

================
Comment at: lib/AST/ASTImporter.cpp:5642
@@ +5641,3 @@
+  return new (Importer.getToContext()) OpaqueValueExpr(
+        Importer.Import(E->getExprLoc()), T, E->getValueKind(),
+        E->getObjectKind(), SourceExpr);
----------------
`getExprLoc` -> `getLocation`.

================
Comment at: lib/AST/ASTImporter.cpp:5769
@@ +5768,3 @@
+  CXXNamedCastExpr *Named = cast<CXXNamedCastExpr>(E);
+  SourceLocation ExprLoc = Importer.Import(Named->getExprLoc()),
+      RParenLoc = Importer.Import(Named->getRParenLoc());
----------------
`getExprLoc` -> `getOperatorLoc`.

================
Comment at: lib/AST/ASTImporter.cpp:5859
@@ +5858,3 @@
+      return nullptr;
+    Exprs[i] = ToIndexExpr;
+  }
----------------
Is there any reason to use subscript instead of `push_back`? If no, `push_back` is more clear.

================
Comment at: lib/AST/ASTImporter.cpp:5890
@@ +5889,3 @@
+
+  // We cannot get source CanThrowVal so correction is required after init
+  CXXNoexceptExpr *ToE = new (Importer.getToContext()) CXXNoexceptExpr(
----------------
This code calculates parameter of type `canThrowResult`:
```
CanThrowResult V;
if (E->isValueDependent)
  V = CT_Dependent;
else
  V = E->getValue() ? CT_Can : CT_Cannot;
```
It looks more clear that the manipulations with created expression, doesn't it?

================
Comment at: lib/AST/ASTImporter.cpp:5909
@@ +5908,3 @@
+  return new (Importer.getToContext()) CXXThrowExpr(
+        SubExpr, T, Importer.Import(E->getExprLoc()),
+        E->isThrownVariableInScope());
----------------
`getExprLoc` -> `getThrowLoc`.

================
Comment at: lib/AST/ASTImporter.cpp:5923
@@ +5922,3 @@
+  return CXXDefaultArgExpr::Create(
+        Importer.getToContext(), Importer.Import(E->getExprLoc()),
+        Param, SubExpr);
----------------
`getExprLoc` -> `getUsedLocation`

================
Comment at: lib/AST/ASTImporter.cpp:5973
@@ +5972,3 @@
+        Importer.getToContext(), T,
+        Importer.Import(CE->getExprLoc()),
+        Ctor,
----------------
`getExprLoc` -> `getParenOrBraceRange`

================
Comment at: lib/AST/ASTImporter.cpp:6002
@@ +6001,3 @@
+        T, TempE, E->isBoundToLvalueReference());
+  ToMTE->setExtendingDecl(ExtendedBy, E->getManglingNumber());
+  return ToMTE;
----------------
Should `ManglingNumber` get numbers associated with 'to' context?


Repository:
  rL LLVM

http://reviews.llvm.org/D14326