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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits