From 6b4e8f82a3f85c6612997e5f9fce8da6f8cb4526 Mon Sep 17 00:00:00 2001 From: Nathan James Date: Tue, 20 Apr 2021 17:13:44 +0100 Subject: [PATCH] [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 --- clang-tools-extra/clangd/ClangdServer.cpp | 10 ++++++---- clang-tools-extra/clangd/refactor/Tweak.cpp | 5 +++-- clang-tools-extra/clangd/refactor/Tweak.h | 6 +++++- .../clangd/refactor/tweaks/DefineOutline.cpp | 12 +++++------- clang-tools-extra/clangd/tool/Check.cpp | 3 ++- .../clangd/unittests/FeatureModulesTests.cpp | 3 ++- .../clangd/unittests/tweaks/TweakTesting.cpp | 13 +++++++++---- 7 files changed, 32 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 86eb53130d55..f36c7d8cdefb 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>> -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( - 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 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 Res; @@ -618,7 +619,8 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID, this](Expected 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> Effect; diff --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp index 8ccb28f236bd..33d43278a5da 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 b381bdb2b799..60ee34d138d6 100644 --- a/clang-tools-extra/clangd/refactor/Tweak.h +++ b/clang-tools-extra/clangd/refactor/Tweak.h @@ -51,7 +51,8 @@ public: /// 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 @@ public: 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 18c521119972..645b4af36a27 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 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 @@ public: 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 dc17530a2f8f..0b1da9677197 100644 --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -210,7 +210,8 @@ public: 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 8611f16688c7..2686c43b3530 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 71bd09e34177..e4637a04a7ed 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 rangeOrPoint(const Annotations &A) { // Returns None if and only if prepare() failed. llvm::Optional> applyTweak(ParsedAST &AST, const Annotations &Input, StringRef TweakID, - const SymbolIndex *Index) { + const SymbolIndex *Index, llvm::vfs::FileSystem *FS) { auto Range = rangeOrPoint(Input); llvm::Optional> 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)