[PATCH] D60821: [clangd] Emit better error messages when rename fails.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 18 03:22:12 PDT 2019

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

LG, thanks!

I do think it can be further simplified, but if not then land as-is.

Comment at: clang-tools-extra/clangd/ClangdServer.cpp:57
+  RefactoringResultCollector(DiagnosticsEngine &DE) : Diags(DE) {}
   void handleError(llvm::Error Err) override {
hokein wrote:
> 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.
But we just store the err in Result, and then ClangdServer::rename does
if (!ResultCollector.Result.getValue())
      return CB(ResultCollector.Result->takeError());
can it wrap the takeError() in expandDiagnostics()?

it still has access to the diagnosticsengine I think

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list