[PATCH] D51633: [ASTImporter] Added error handling for AST import.

[Aleksei Sidorin via Phabricator via cfe-commits]

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