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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D36969#848518, @arphaman wrote:

> 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.


Although I guess I could always change the approach completely and use variadic templates instead of `<<`, e.g.:

  template<typename... T>
  Error RefactoringOperation::error(SourceLocation Loc, unsigned DiagId, const T &... Arguments) {
    return llvm::make_error<DiagnosticError>(...);
  }
  
  DiagnosticOr<...> function(RefactoringOperation &Op) {
     return Op.error(Loc, diag::err_something, "invalid");
  }

I think I'll try that instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D36969