[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hello Takafumi,

Thank you for this patch! I feel positive about it. You can find my comments inline.



================
Comment at: lib/AST/ASTImporter.cpp:5540
+  for(auto FromArg : E->getArgs()) {
+    TypeSourceInfo *ToTI = Importer.Import(FromArg);
+    ToArgVec.push_back(ToTI);
----------------
We should fail (`return nullptr`) if import fails and `ToTI` is `nullptr`.


================
Comment at: lib/AST/ASTImporter.cpp:5543
+  }
+  ArrayRef<TypeSourceInfo *> ToArgRef(ToArgVec);
+
----------------
No need to create an ArrayRef - SmallVector can be implicitly converted to it so you can just pass SmallVector to the function.


================
Comment at: lib/AST/ASTImporter.cpp:5545
+
+  return TypeTraitExpr::Create( Importer.getToContext(),
+                         ToType,
----------------
The style looks a bit broken. Could you clang-format?


================
Comment at: lib/AST/ASTImporter.cpp:5551
+                         Importer.Import(E->getLocEnd()),
+                         E->getValue());
+}
----------------
This will assert if E is value-dependent. You need to check it explicitly.


https://reviews.llvm.org/D39722