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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
