hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:57
public:
+ RefactoringResultCollector(DiagnosticsEngine &DE) : Diags(DE) {}
void handleError(llvm::Error Err) override {
----------------
sammccall wrote:
> why inject the DE here (and handle mapping errors both in the caller and
> here) rather than always doing the mapping in the caller?
We don't have control of the Err caller for this class (the Error is passed via
the `handleError` interface)...so we need the DE in this class to do the
expanding.
================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:315
+ auto Error = Rename.takeError();
+ if (auto Diag = DiagnosticError::take(Error)) {
+ llvm::cantFail(std::move(Error));
----------------
sammccall wrote:
> can we avoid this duplication by giving a helper function more
> responsibilities?
> e.g. `llvm::Error expandDiagnostics(llvm::Error, DiagnosticsEngine&)`?
SG!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60821/new/
https://reviews.llvm.org/D60821
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
