[PATCH] D108268: [Modules] Change result of reading AST block to llvm::Error instead

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai accepted this revision.
vsapsai added a comment.
This revision is now accepted and ready to land.

Allowing multiple PartialDiagnostic to be in-flight/delayed is a nice goal but it's not clear if it's worth it. Also I'm not sure it is worth investing into making delayed diagnostic easier to use. But that is a separate change and the current one looks fine.



================
Comment at: clang/lib/Serialization/ASTReader.cpp:1268
+  SmallVector<std::string, 2> Errors;
+  handleAllErrors(
+      std::move(Err),
----------------
Don't have a strong opinion but an alternative to preserve more of the old code and avoid manual SmallVector manipulations can be

```lang=c++
llvm::Error RemainingErr = handleErrors(std::move(Err), /*DiagnosticError lambda*/);
if (RemainingErr)
  Error(toString(std::move(RemainingErr)));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108268