[PATCH] D36969: [Basic] Add a DiagnosticOr type

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D36969#847803, @vsk wrote:

> Would it be more convenient to extend ErrorInfo instead of creating a new diagnostic wrapper? E.g one benefit of passing around Expected<T>'s is that there's some assurance the diagnostics will be reported.


Possibly, but one issue I found with ErrorInfo is that it gets harder to report diagnostics. E.g. with `DiagnosticOr` you will be able to write:

  DiagnosticOr<...> function(RefactoringOperation &Op) {
    return Op.Diag(Loc, err::something) << "invalid";
  }

But with `Expected` you'll have to use something like:

  Expected<...> function(RefactoringOperation &Op) {
    return llvm::make_error<DiagnosticError>(Op.Diag(Loc, err::something) << "invalid");
  }

I don't think that the assurance about reporting these diagnostics is that important for my case. They will be consumed at most in 3 places (clang-refactor, clangd, and libclang) and clangd and it will obvious to the user if errors aren't getting reported.


Repository:
  rL LLVM

https://reviews.llvm.org/D36969