a_sidorin added a comment. Hi Balázs,
Finally, I have completed the review. Despite the fact that this patch is huge, I found only a few issues. ================ Comment at: lib/AST/ASTImporter.cpp:4604 + if (Error Err = ImportDeclContext(D)) + // FIXME: Really ignore the error? + consumeError(std::move(Err)); ---------------- I think we can just `return std::move(Err);` as it was done below. ================ Comment at: lib/AST/ASTImporter.cpp:5657 + if (!ToSemiLocOrErr) + return nullptr; + return new (Importer.getToContext()) NullStmt( ---------------- Shouldn't we return an error here? ================ Comment at: lib/AST/ASTImporter.cpp:7339 + // FIXME: Why is this copy needed? + // Why not used in the CXXOperatorCallExpr case? auto **ToArgs_Copied = new (Importer.getToContext()) Expr*[NumArgs]; ---------------- That's true, this copying can be removed. Repository: rC Clang https://reviews.llvm.org/D51633 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
