[PATCH] D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hi Peter,

Thank you for the patch. You can find some comments inline.



================
Comment at: lib/AST/ASTImporter.cpp:5476
+
+  for (unsigned ai = 0, ae = NumArgs; ai != ae; ++ai) {
+    Expr *FromArg = CE->getArg(ai);
----------------
xazax.hun wrote:
> Use uppercase variable names. 
We have implemented ImportContainerChecked helper to avoid such routine code. You can use it.


================
Comment at: lib/AST/ASTImporter.cpp:5484
+
+  Expr **ToArgs_Copied = new (Importer.getToContext()) Expr *[NumArgs];
+
----------------
No need to allocate and copy - CXXUnresolvedConstructExpr::Create allocates the required amount of memory itself.


================
Comment at: lib/AST/ASTImporter.cpp:5510
+  UnresolvedSet<8> ToDecls;
+  for (Decl *D : E->decls()) {
+    if (NamedDecl *To = cast_or_null<NamedDecl>(Importer.Import(D)))
----------------
ImportContainerChecked


================
Comment at: lib/AST/ASTImporter.cpp:5520
+  if (E->hasExplicitTemplateArgs()) {
+    for (const auto &FromLoc : E->template_arguments()) {
+      bool Error = false;
----------------
Needs to become a common function.


================
Comment at: unittests/AST/ASTImporterTest.cpp:521
+                 "template <typename T> void declToImport() {"
+                 "C<T> d;"
+                 "d.t = T()"
----------------
Code samples need alignment.


https://reviews.llvm.org/D38694