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

[Ben Barham via Phabricator via cfe-commits]

bnbarham added a comment.

In D108268#2958568 <https://reviews.llvm.org/D108268#2958568>, @vsapsai wrote:

> `DiagnosticError` looks like a good fit for the task at hand, so it is worth to try it. Though I don't know if it would end up in the end convoluted or OK.

Unless we also change `DiagnosticEngine` it doesn't look like this is a viable solution. The `PartialDiagnostic` can't be emitted straight to `Diags`, since there may already be a diagnostic in flight (see `Error(unsigned DiagID, ...)`). The args in `PartialDiagnostic` are currently protected and it seems weird to change that, but even if they weren't it sort of defeats the purpose of using `DiagnosticError` in the first place.

Any other ideas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108268