diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -392,7 +392,6 @@ // Store of the current versions of the open documents. // Only written from the main thread (despite being threadsafe). - // FIXME: TUScheduler also keeps these, unify? DraftStore DraftMgr; const std::unique_ptr DirtyFS; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -484,15 +484,16 @@ const RenameOptions &RenameOpts, Callback CB) { auto Action = [Pos, File = File.str(), CB = std::move(CB), - NewName = std::move(NewName), + NewName = std::move(NewName), this, RenameOpts](llvm::Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); // prepareRename is latency-sensitive: we don't query the index, as we // only need main-file references - auto Results = clangd::rename( - {Pos, NewName.getValueOr("__clangd_rename_dummy"), InpAST->AST, File, - /*Index=*/nullptr, RenameOpts}); + auto Results = + clangd::rename({Pos, NewName.getValueOr("__clangd_rename_dummy"), + InpAST->AST, File, TFS.view(llvm::None), + /*Index=*/nullptr, RenameOpts}); if (!Results) { // LSP says to return null on failure, but that will result in a generic // failure message. If we send an LSP error response, clients can surface @@ -507,25 +508,16 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName, const RenameOptions &Opts, Callback CB) { - // A snapshot of all file dirty buffers. - llvm::StringMap Snapshot = WorkScheduler->getAllFileContents(); auto Action = [File = File.str(), NewName = NewName.str(), Pos, Opts, - CB = std::move(CB), Snapshot = std::move(Snapshot), + CB = std::move(CB), this](llvm::Expected InpAST) mutable { // Tracks number of files edited per invocation. static constexpr trace::Metric RenameFiles("rename_files", trace::Metric::Distribution); if (!InpAST) return CB(InpAST.takeError()); - auto GetDirtyBuffer = - [&Snapshot](PathRef AbsPath) -> llvm::Optional { - auto It = Snapshot.find(AbsPath); - if (It == Snapshot.end()) - return llvm::None; - return It->second; - }; - auto R = clangd::rename( - {Pos, NewName, InpAST->AST, File, Index, Opts, GetDirtyBuffer}); + auto R = clangd::rename({Pos, NewName, InpAST->AST, File, + DirtyFS->view(llvm::None), Index, Opts}); if (!R) return CB(R.takeError()); diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -235,9 +235,6 @@ /// if requested with WantDiags::Auto or WantDiags::Yes. void remove(PathRef File); - /// Returns a snapshot of all file buffer contents, per last update(). - llvm::StringMap getAllFileContents() const; - /// Schedule an async task with no dependencies. /// Path may be empty (it is used only to set the Context). void run(llvm::StringRef Name, llvm::StringRef Path, diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -1519,13 +1519,6 @@ // preamble of several open headers. } -llvm::StringMap TUScheduler::getAllFileContents() const { - llvm::StringMap Results; - for (auto &It : Files) - Results.try_emplace(It.getKey(), It.getValue()->Contents); - return Results; -} - void TUScheduler::run(llvm::StringRef Name, llvm::StringRef Path, llvm::unique_function Action) { runWithSemaphore(Name, Path, std::move(Action), Barrier); diff --git a/clang-tools-extra/clangd/refactor/Rename.h b/clang-tools-extra/clangd/refactor/Rename.h --- a/clang-tools-extra/clangd/refactor/Rename.h +++ b/clang-tools-extra/clangd/refactor/Rename.h @@ -21,11 +21,6 @@ class ParsedAST; class SymbolIndex; -/// Gets dirty buffer for a given file \p AbsPath. -/// Returns None if there is no dirty buffer for the given file. -using DirtyBufferGetter = - llvm::function_ref(PathRef AbsPath)>; - struct RenameOptions { /// The maximum number of affected files (0 means no limit), only meaningful /// when AllowCrossFile = true. @@ -42,14 +37,12 @@ ParsedAST &AST; llvm::StringRef MainFilePath; + // The filesystem to query when performing cross file renames. + llvm::IntrusiveRefCntPtr FS; + const SymbolIndex *Index = nullptr; RenameOptions Opts = {}; - // When set, used by the rename to get file content for all rename-related - // files. - // If there is no corresponding dirty buffer, we will use the file content - // from disk. - DirtyBufferGetter GetDirtyBuffer = nullptr; }; struct RenameResult { diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -586,10 +586,10 @@ // index (background index) is relatively stale. We choose the dirty buffers // as the file content we rename on, and fallback to file content on disk if // there is no dirty buffer. -llvm::Expected renameOutsideFile( - const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, - llvm::StringRef NewName, const SymbolIndex &Index, size_t MaxLimitFiles, - llvm::function_ref(PathRef)> GetFileContent) { +llvm::Expected +renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, + llvm::StringRef NewName, const SymbolIndex &Index, + size_t MaxLimitFiles, llvm::vfs::FileSystem &FS) { trace::Span Tracer("RenameOutsideFile"); auto AffectedFiles = findOccurrencesOutsideFile(RenameDecl, MainFilePath, Index, MaxLimitFiles); @@ -599,13 +599,16 @@ for (auto &FileAndOccurrences : *AffectedFiles) { llvm::StringRef FilePath = FileAndOccurrences.first(); - auto AffectedFileCode = GetFileContent(FilePath); - if (!AffectedFileCode) { - elog("Fail to read file content: {0}", AffectedFileCode.takeError()); + auto ExpBuffer = FS.getBufferForFile(FilePath); + if (!ExpBuffer) { + elog("Fail to read file content: Fail to open file {0}: {1}", FilePath, + ExpBuffer.getError().message()); continue; } + + auto AffectedFileCode = (*ExpBuffer)->getBuffer(); auto RenameRanges = - adjustRenameRanges(*AffectedFileCode, RenameDecl.getNameAsString(), + adjustRenameRanges(AffectedFileCode, RenameDecl.getNameAsString(), std::move(FileAndOccurrences.second), RenameDecl.getASTContext().getLangOpts()); if (!RenameRanges) { @@ -617,7 +620,7 @@ FilePath); } auto RenameEdit = - buildRenameEdit(FilePath, *AffectedFileCode, *RenameRanges, NewName); + buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName); if (!RenameEdit) return error("failed to rename in file {0}: {1}", FilePath, RenameEdit.takeError()); @@ -673,23 +676,6 @@ ParsedAST &AST = RInputs.AST; const SourceManager &SM = AST.getSourceManager(); llvm::StringRef MainFileCode = SM.getBufferData(SM.getMainFileID()); - auto GetFileContent = [&RInputs, - &SM](PathRef AbsPath) -> llvm::Expected { - llvm::Optional DirtyBuffer; - if (RInputs.GetDirtyBuffer && - (DirtyBuffer = RInputs.GetDirtyBuffer(AbsPath))) - return std::move(*DirtyBuffer); - - auto Content = - SM.getFileManager().getVirtualFileSystem().getBufferForFile(AbsPath); - if (!Content) - return error("Fail to open file {0}: {1}", AbsPath, - Content.getError().message()); - if (!*Content) - return error("Got no buffer for file {0}", AbsPath); - - return (*Content)->getBuffer().str(); - }; // Try to find the tokens adjacent to the cursor position. auto Loc = sourceLocationInMainFile(SM, RInputs.Pos); if (!Loc) @@ -765,7 +751,7 @@ RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index, Opts.LimitFiles == 0 ? std::numeric_limits::max() : Opts.LimitFiles, - GetFileContent); + *RInputs.FS); if (!OtherFilesEdits) return OtherFilesEdits.takeError(); Result.GlobalChanges = *OtherFilesEdits; diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -33,6 +33,19 @@ using testing::UnorderedElementsAre; using testing::UnorderedElementsAreArray; +llvm::IntrusiveRefCntPtr +createOverlay(llvm::IntrusiveRefCntPtr Base, + llvm::IntrusiveRefCntPtr Overlay) { + auto OFS = + llvm::makeIntrusiveRefCnt(std::move(Base)); + OFS->pushOverlay(std::move(Overlay)); + return OFS; +} + +llvm::IntrusiveRefCntPtr getVFSFromAST(ParsedAST &AST) { + return &AST.getSourceManager().getFileManager().getVirtualFileSystem(); +} + // Convert a Range to a Ref. Ref refWithRange(const clangd::Range &Range, const std::string &URI) { Ref Result; @@ -815,7 +828,8 @@ auto Index = TU.index(); for (const auto &RenamePos : Code.points()) { auto RenameResult = - rename({RenamePos, NewName, AST, testPath(TU.Filename), Index.get()}); + rename({RenamePos, NewName, AST, testPath(TU.Filename), + getVFSFromAST(AST), Index.get()}); ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError(); ASSERT_EQ(1u, RenameResult->GlobalChanges.size()); EXPECT_EQ( @@ -1072,7 +1086,8 @@ } auto AST = TU.build(); llvm::StringRef NewName = Case.NewName; - auto Results = rename({T.point(), NewName, AST, testPath(TU.Filename)}); + auto Results = rename( + {T.point(), NewName, AST, testPath(TU.Filename), getVFSFromAST(AST)}); bool WantRename = true; if (T.ranges().empty()) WantRename = false; @@ -1101,13 +1116,20 @@ auto AST = TU.build(); auto Main = testPath("main.cc"); + auto InMemFS = llvm::makeIntrusiveRefCnt(); + InMemFS->addFile(testPath("main.cc"), 0, + llvm::MemoryBuffer::getMemBuffer(Code.code())); + InMemFS->addFile(testPath("other.cc"), 0, + llvm::MemoryBuffer::getMemBuffer(Code.code())); auto Rename = [&](const SymbolIndex *Idx) { - auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional { - return Code.code().str(); // Every file has the same content. - }; - RenameInputs Inputs{Code.point(), "xPrime", AST, Main, - Idx, RenameOptions(), GetDirtyBuffer}; + RenameInputs Inputs{Code.point(), + "xPrime", + AST, + Main, + createOverlay(getVFSFromAST(AST), InMemFS), + Idx, + RenameOptions()}; auto Results = rename(Inputs); EXPECT_TRUE(bool(Results)) << llvm::toString(Results.takeError()); return std::move(*Results); @@ -1156,8 +1178,8 @@ auto AST = TU.build(); llvm::StringRef NewName = "abcde"; - auto RenameResult = - rename({Code.point(), NewName, AST, testPath(TU.Filename)}); + auto RenameResult = rename( + {Code.point(), NewName, AST, testPath(TU.Filename), getVFSFromAST(AST)}); ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << Code.point(); ASSERT_EQ(1u, RenameResult->GlobalChanges.size()); EXPECT_EQ(applyEdits(std::move(RenameResult->GlobalChanges)).front().second, @@ -1173,7 +1195,8 @@ )cpp"; TU.HeaderFilename = "protobuf.pb.h"; auto AST = TU.build(); - auto Results = rename({Code.point(), "newName", AST, testPath(TU.Filename)}); + auto Results = rename({Code.point(), "newName", AST, testPath(TU.Filename), + getVFSFromAST(AST)}); EXPECT_FALSE(Results); EXPECT_THAT(llvm::toString(Results.takeError()), testing::HasSubstr("not a supported kind")); @@ -1237,25 +1260,19 @@ Annotations MainCode("class [[Fo^o]] {};"); auto MainFilePath = testPath("main.cc"); - // Dirty buffer for foo.cc. - auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional { - if (Path == FooPath) - return FooDirtyBuffer.code().str(); - return llvm::None; - }; + llvm::IntrusiveRefCntPtr InMemFS = + new llvm::vfs::InMemoryFileSystem; + InMemFS->addFile(FooPath, 0, + llvm::MemoryBuffer::getMemBuffer(FooDirtyBuffer.code())); // Run rename on Foo, there is a dirty buffer for foo.cc, rename should // respect the dirty buffer. TestTU TU = TestTU::withCode(MainCode.code()); auto AST = TU.build(); llvm::StringRef NewName = "newName"; - auto Results = rename({MainCode.point(), - NewName, - AST, - MainFilePath, - Index.get(), - {}, - GetDirtyBuffer}); + auto Results = + rename({MainCode.point(), NewName, AST, MainFilePath, + createOverlay(getVFSFromAST(AST), InMemFS), Index.get()}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(Results->GlobalChanges)), @@ -1270,13 +1287,8 @@ // Set a file "bar.cc" on disk. TU.AdditionalFiles["bar.cc"] = std::string(BarCode.code()); AST = TU.build(); - Results = rename({MainCode.point(), - NewName, - AST, - MainFilePath, - Index.get(), - {}, - GetDirtyBuffer}); + Results = rename({MainCode.point(), NewName, AST, MainFilePath, + createOverlay(getVFSFromAST(AST), InMemFS), Index.get()}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(Results->GlobalChanges)), @@ -1312,13 +1324,8 @@ size_t estimateMemoryUsage() const override { return 0; } } PIndex; - Results = rename({MainCode.point(), - NewName, - AST, - MainFilePath, - &PIndex, - {}, - GetDirtyBuffer}); + Results = rename({MainCode.point(), NewName, AST, MainFilePath, + createOverlay(getVFSFromAST(AST), InMemFS), &PIndex}); EXPECT_FALSE(Results); EXPECT_THAT(llvm::toString(Results.takeError()), testing::HasSubstr("too many occurrences")); @@ -1368,8 +1375,8 @@ Ref ReturnedRef; } DIndex(XRefInBarCC); llvm::StringRef NewName = "newName"; - auto Results = - rename({MainCode.point(), NewName, AST, MainFilePath, &DIndex}); + auto Results = rename({MainCode.point(), NewName, AST, MainFilePath, + getVFSFromAST(AST), &DIndex}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(Results->GlobalChanges)), @@ -1588,7 +1595,7 @@ auto Path = testPath(TU.Filename); auto AST = TU.build(); llvm::StringRef NewName = "newName"; - auto Results = rename({Code.point(), NewName, AST, Path}); + auto Results = rename({Code.point(), NewName, AST, Path, getVFSFromAST(AST)}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(Results->GlobalChanges)),