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 @@ -394,7 +394,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; 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 @@ -490,9 +490,10 @@ 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, /*FS=*/nullptr, + /*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,14 @@ ParsedAST &AST; llvm::StringRef MainFilePath; + // The filesystem to query when performing cross file renames. + // If this is set, Index must also be set, likewise if this is nullptr, Index + // must also be nullptr. + llvm::IntrusiveRefCntPtr FS = nullptr; + 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()); @@ -668,28 +671,13 @@ } // namespace llvm::Expected rename(const RenameInputs &RInputs) { + assert(!RInputs.Index == !RInputs.FS && + "Index and FS must either both be specified or both null."); trace::Span Tracer("Rename flow"); const auto &Opts = RInputs.Opts; 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 +753,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( @@ -1101,13 +1115,21 @@ 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, + Idx ? createOverlay(getVFSFromAST(AST), InMemFS) + : nullptr, + Idx, + RenameOptions()}; auto Results = rename(Inputs); EXPECT_TRUE(bool(Results)) << llvm::toString(Results.takeError()); return std::move(*Results); @@ -1237,25 +1259,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 +1286,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 +1323,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 +1374,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)),