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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:764
+      /*WantFormat=*/true,
+      [this](PathRef File) { return DraftMgr.getDraft(File); },
+      [File, Params, Reply = std::move(Reply),
----------------
ilya-biryukov wrote:
> We should probably get a snapshot of all dirty buffers here instead.
> A racy  way (rename is run on a separate thread, new files updates might come in in the meantime) to get contents of the file looks like a bad idea, this will lead to hard-to-debug failures...
> 
> Note that `ClangdServer` also has access to all contents of all the files, they are stored in `TUScheduler`, so we don't need to pass `GetDirtyBuffer` callback up until the final run of `rename`
thanks, I like the idea. This could simplify the code, with this approach, the dirty buffer of the main file would be the same as the one used for building the AST.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263