[PATCH] D69263: [clangd] Implement cross-file rename.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM.

It's probably worth collecting a list of things we need to fix before enabling cross-file rename and putting it somewhere (a GitHub issue, maybe?)
Important things that immediately come to mind:

- add range-patching heuristics, measure how good they are
- avoid `O(N^2)` when computing edits (converting from positions to offsets)
- handle implicit references from the index

There are definitely more.



================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:345
+  SourceLocation SourceLocationBeg =
+      SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
+          RInputs.Pos, SM, AST.getASTContext().getLangOpts()));
----------------
hokein wrote:
> ilya-biryukov wrote:
> > Why is this different from `prepareRename`, which does not call `getMacroArgExpandedLocation`?
> > 
> I didn't change it in this patch, but you raise a good point, `prepareRename` should call `getMacroArgExpandedLocation`.
Yep, makes sense to change it later. Could you put a FIXME, so that we don't forget about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263