[PATCH] D65936: [clangd] Use raw rename functions to implement the rename.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:182
+            CharSourceRange::getCharRange(Rename.getNameRanges()[0]), NewName)))
+      return std::move(Err);
   }
----------------
hokein wrote:
> sammccall wrote:
> > for actual error handling behavior (if this can actually fail): is this a deliberate choice to refuse to perform any of the edits if there are conflicts? Is this better than applying some non-conflicting set?
> > 
> > Unlike most error-propagation, this is a nontrivial policy choice and needs a comment.
> I think if there are conflicts, it is a signal that indicates we have bugs in the code (either in clangd, or rename library), applying parts of them may be not valuable (like generating uncompilable code)
> 
> I'd prefer to refuse to perform renaming if we have conflicts, WDYT?
That sounds OK to me, can you add a comment?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65936/new/

https://reviews.llvm.org/D65936