martong added a comment.

Thanks for the patch! It look almost good to me, but I have a comment about the 
error handling.



================
Comment at: clang/lib/AST/ASTImporter.cpp:1707
+            if (Err)
+              return Err;
+            }
----------------
Rather than just simply returning with the Error, I think we should append this 
error to `ChildErrors` and go on with the next field, as this is what the 
original code did.
I am thinking about something like this:
```
      if (Err && AccumulateChildErrors)
        ChildErrors =  joinErrors(std::move(ChildErrors), Err);
      else
        consumeError(Err);
```

And perhaps line 1713 could be just a simple `else {`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71378/new/

https://reviews.llvm.org/D71378



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to