[PATCH] D95043: [clangd] Use Dirty Filesystem for cross file rename.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 9 00:21:27 PST 2021


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

This looks pretty good, great cleanup!



================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:408
+        clangd::rename({Pos, NewName.getValueOr("__clangd_rename_dummy"),
+                        InpAST->AST, File, TFS.view(llvm::None),
+                        /*Index=*/nullptr, RenameOpts});
----------------
we shouldn't need to pass a nontrivial FS in here, right?


================
Comment at: clang-tools-extra/clangd/refactor/Rename.h:40
 
+  // The filesystem to query when performing cross file renames.
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
----------------
Index + FS are both only used for cross-file renames at this point.

(We used to support a single-file rename mode where the index was used to validate that the symbol wasn't used elsewhere, but that's gone now)

I think we should document this, and assert that they're either both set (cross-file) or neither is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95043



More information about the cfe-commits mailing list