[clang-tools-extra] 6b4e8f8 - [clangd] Use dirty filesystem when performing cross file tweaks

Nathan James via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 20 09:13:56 PDT 2021


Author: Nathan James
Date: 2021-04-20T17:13:44+01:00
New Revision: 6b4e8f82a3f85c6612997e5f9fce8da6f8cb4526

URL: https://github.com/llvm/llvm-project/commit/6b4e8f82a3f85c6612997e5f9fce8da6f8cb4526
DIFF: https://github.com/llvm/llvm-project/commit/6b4e8f82a3f85c6612997e5f9fce8da6f8cb4526.diff

LOG: [clangd] Use dirty filesystem when performing cross file tweaks

Cross file tweaks can now use the dirty buffer contents easily when performing cross file effects.
This can be noted on the DefineOutline tweak, now working when the target file is unsaved

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D93978

Added: 
    

Modified: 
    clang-tools-extra/clangd/ClangdServer.cpp
    clang-tools-extra/clangd/refactor/Tweak.cpp
    clang-tools-extra/clangd/refactor/Tweak.h
    clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
    clang-tools-extra/clangd/tool/Check.cpp
    clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
    clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 86eb53130d55d..f36c7d8cdefb7 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -549,7 +549,8 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
 // May generate several candidate selections, due to SelectionTree ambiguity.
 // vector of pointers because GCC doesn't like non-copyable Selection.
 static llvm::Expected<std::vector<std::unique_ptr<Tweak::Selection>>>
-tweakSelection(const Range &Sel, const InputsAndAST &AST) {
+tweakSelection(const Range &Sel, const InputsAndAST &AST,
+               llvm::vfs::FileSystem *FS) {
   auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start);
   if (!Begin)
     return Begin.takeError();
@@ -561,7 +562,7 @@ tweakSelection(const Range &Sel, const InputsAndAST &AST) {
       AST.AST.getASTContext(), AST.AST.getTokens(), *Begin, *End,
       [&](SelectionTree T) {
         Result.push_back(std::make_unique<Tweak::Selection>(
-            AST.Inputs.Index, AST.AST, *Begin, *End, std::move(T)));
+            AST.Inputs.Index, AST.AST, *Begin, *End, std::move(T), FS));
         return false;
       });
   assert(!Result.empty() && "Expected at least one SelectionTree");
@@ -580,7 +581,7 @@ void ClangdServer::enumerateTweaks(
                     Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
-    auto Selections = tweakSelection(Sel, *InpAST);
+    auto Selections = tweakSelection(Sel, *InpAST, /*FS=*/nullptr);
     if (!Selections)
       return CB(Selections.takeError());
     std::vector<TweakRef> Res;
@@ -618,7 +619,8 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
                  this](Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
-    auto Selections = tweakSelection(Sel, *InpAST);
+    auto FS = DirtyFS->view(llvm::None);
+    auto Selections = tweakSelection(Sel, *InpAST, FS.get());
     if (!Selections)
       return CB(Selections.takeError());
     llvm::Optional<llvm::Expected<Tweak::Effect>> Effect;

diff  --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp
index 8ccb28f236bd1..33d43278a5dad 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.cpp
+++ b/clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -61,9 +61,10 @@ getAllTweaks(const FeatureModuleSet *Modules) {
 
 Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
                             unsigned RangeBegin, unsigned RangeEnd,
-                            SelectionTree ASTSelection)
+                            SelectionTree ASTSelection,
+                            llvm::vfs::FileSystem *FS)
     : Index(Index), AST(&AST), SelectionBegin(RangeBegin),
-      SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)) {
+      SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)), FS(FS) {
   auto &SM = AST.getSourceManager();
   Code = SM.getBufferData(SM.getMainFileID());
   Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin);

diff  --git a/clang-tools-extra/clangd/refactor/Tweak.h b/clang-tools-extra/clangd/refactor/Tweak.h
index b381bdb2b799b..60ee34d138d6b 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.h
+++ b/clang-tools-extra/clangd/refactor/Tweak.h
@@ -51,7 +51,8 @@ class Tweak {
   /// Input to prepare and apply tweaks.
   struct Selection {
     Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
-              unsigned RangeEnd, SelectionTree ASTSelection);
+              unsigned RangeEnd, SelectionTree ASTSelection,
+              llvm::vfs::FileSystem *VFS);
     /// The text of the active document.
     llvm::StringRef Code;
     /// The Index for handling codebase related queries.
@@ -67,6 +68,9 @@ class Tweak {
     unsigned SelectionEnd;
     /// The AST nodes that were selected.
     SelectionTree ASTSelection;
+    /// File system used to access source code (for cross-file tweaks).
+    /// This is only populated when applying a tweak, not during prepare.
+    llvm::vfs::FileSystem *FS = nullptr;
     // FIXME: provide a way to get sources and ASTs for other files.
   };
 

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 18c521119972b..645b4af36a27c 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -64,9 +64,8 @@ const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) {
 
 llvm::Optional<Path> getSourceFile(llvm::StringRef FileName,
                                    const Tweak::Selection &Sel) {
-  if (auto Source = getCorrespondingHeaderOrSource(
-          FileName,
-          &Sel.AST->getSourceManager().getFileManager().getVirtualFileSystem()))
+  assert(Sel.FS);
+  if (auto Source = getCorrespondingHeaderOrSource(FileName, Sel.FS))
     return *Source;
   return getCorrespondingHeaderOrSource(FileName, *Sel.AST, Sel.Index);
 }
@@ -414,12 +413,11 @@ class DefineOutline : public Tweak {
       return error("Couldn't get absolute path for main file.");
 
     auto CCFile = getSourceFile(*MainFileName, Sel);
+
     if (!CCFile)
       return error("Couldn't find a suitable implementation file.");
-
-    auto &FS =
-        Sel.AST->getSourceManager().getFileManager().getVirtualFileSystem();
-    auto Buffer = FS.getBufferForFile(*CCFile);
+    assert(Sel.FS && "FS Must be set in apply");
+    auto Buffer = Sel.FS->getBufferForFile(*CCFile);
     // FIXME: Maybe we should consider creating the implementation file if it
     // doesn't exist?
     if (!Buffer)

diff  --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
index dc17530a2f8f5..0b1da96771972 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -210,7 +210,8 @@ class Checker {
       vlog("  {0} {1}", Pos, Tok.text(AST->getSourceManager()));
       auto Tree = SelectionTree::createRight(AST->getASTContext(),
                                              AST->getTokens(), Start, End);
-      Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree));
+      Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree),
+                                 nullptr);
       for (const auto &T :
            prepareTweaks(Selection, Opts.TweakFilter, Opts.FeatureModules)) {
         auto Result = T->apply(Selection);

diff  --git a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
index 8611f16688c7f..2686c43b3530b 100644
--- a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
@@ -47,7 +47,8 @@ TEST(FeatureModulesTest, ContributesTweak) {
   auto Tree =
       SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), 0, 0);
   auto Actual = prepareTweak(
-      TweakID, Tweak::Selection(nullptr, AST, 0, 0, std::move(Tree)), &Set);
+      TweakID, Tweak::Selection(nullptr, AST, 0, 0, std::move(Tree), nullptr),
+      &Set);
   ASSERT_TRUE(bool(Actual));
   EXPECT_EQ(Actual->get()->id(), TweakID);
 }

diff  --git a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
index 71bd09e341772..e4637a04a7edd 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
@@ -68,13 +68,14 @@ std::pair<unsigned, unsigned> rangeOrPoint(const Annotations &A) {
 // Returns None if and only if prepare() failed.
 llvm::Optional<llvm::Expected<Tweak::Effect>>
 applyTweak(ParsedAST &AST, const Annotations &Input, StringRef TweakID,
-           const SymbolIndex *Index) {
+           const SymbolIndex *Index, llvm::vfs::FileSystem *FS) {
   auto Range = rangeOrPoint(Input);
   llvm::Optional<llvm::Expected<Tweak::Effect>> Result;
   SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first,
                             Range.second, [&](SelectionTree ST) {
                               Tweak::Selection S(Index, AST, Range.first,
-                                                 Range.second, std::move(ST));
+                                                 Range.second, std::move(ST),
+                                                 FS);
                               if (auto T = prepareTweak(TweakID, S, nullptr)) {
                                 Result = (*T)->apply(S);
                                 return true;
@@ -98,7 +99,9 @@ MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
   TU.ExtraArgs = ExtraArgs;
   TU.AdditionalFiles = std::move(ExtraFiles);
   ParsedAST AST = TU.build();
-  auto Result = applyTweak(AST, Input, TweakID, Index);
+  auto Result = applyTweak(
+      AST, Input, TweakID, Index,
+      &AST.getSourceManager().getFileManager().getVirtualFileSystem());
   // We only care if prepare() succeeded, but must handle Errors.
   if (Result && !*Result)
     consumeError(Result->takeError());
@@ -119,7 +122,9 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode,
   TU.ExtraArgs = ExtraArgs;
   ParsedAST AST = TU.build();
 
-  auto Result = applyTweak(AST, Input, TweakID, Index.get());
+  auto Result = applyTweak(
+      AST, Input, TweakID, Index.get(),
+      &AST.getSourceManager().getFileManager().getVirtualFileSystem());
   if (!Result)
     return "unavailable";
   if (!*Result)


        


More information about the cfe-commits mailing list