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

Alex Lorenz via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 22 04:50:42 PDT 2017

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.



More information about the cfe-commits mailing list