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 @@ -197,8 +197,6 @@ } if (DynamicIdx) AddIndex(DynamicIdx.get()); - // Suppress warnings of this being unused, will be used later. - (void)this->DirtyFS; } void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents, @@ -372,6 +370,7 @@ // main-file occurrences; auto Results = clangd::rename( {Pos, NewName.getValueOr("__clangd_rename_dummy"), InpAST->AST, File, + (RenameOpts.AllowCrossFile ? DirtyFS : TFS).view(llvm::None), RenameOpts.AllowCrossFile ? nullptr : Index, RenameOpts}); if (!Results) { // LSP says to return null on failure, but that will result in a generic @@ -387,25 +386,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 @@ -239,9 +239,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 @@ -1330,13 +1330,6 @@ File); } -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) { if (!PreambleTasks) { 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 { /// If true, enable cross-file rename; otherwise, only allows to rename a /// symbol that's only used in the current file. @@ -45,14 +40,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 @@ -509,10 +509,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); @@ -522,13 +522,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) { @@ -540,7 +543,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()); @@ -596,23 +599,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) @@ -689,7 +675,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; @@ -833,8 +846,8 @@ TU.ExtraArgs.push_back("-xobjective-c++"); auto AST = TU.build(); for (const auto &RenamePos : Code.points()) { - auto RenameResult = - rename({RenamePos, NewName, AST, testPath(TU.Filename)}); + auto RenameResult = rename( + {RenamePos, NewName, AST, testPath(TU.Filename), getVFSFromAST(AST)}); ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError(); ASSERT_EQ(1u, RenameResult->GlobalChanges.size()); EXPECT_EQ( @@ -1040,8 +1053,8 @@ } auto AST = TU.build(); llvm::StringRef NewName = Case.NewName; - auto Results = - rename({T.point(), NewName, AST, testPath(TU.Filename), Case.Index}); + auto Results = rename({T.point(), NewName, AST, testPath(TU.Filename), + getVFSFromAST(AST), Case.Index}); bool WantRename = true; if (T.ranges().empty()) WantRename = false; @@ -1081,8 +1094,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, @@ -1098,7 +1111,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")); @@ -1170,12 +1184,10 @@ 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. @@ -1186,9 +1198,9 @@ NewName, AST, MainFilePath, + createOverlay(getVFSFromAST(AST), InMemFS), Index.get(), - {/*CrossFile=*/true}, - GetDirtyBuffer}); + {/*CrossFile=*/true}}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(Results->GlobalChanges)), @@ -1207,9 +1219,9 @@ NewName, AST, MainFilePath, + createOverlay(getVFSFromAST(AST), InMemFS), Index.get(), - {/*CrossFile=*/true}, - GetDirtyBuffer}); + {/*CrossFile=*/true}}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(Results->GlobalChanges)), @@ -1249,9 +1261,9 @@ NewName, AST, MainFilePath, + createOverlay(getVFSFromAST(AST), InMemFS), &PIndex, - {/*CrossFile=*/true}, - GetDirtyBuffer}); + {/*CrossFile=*/true}}); EXPECT_FALSE(Results); EXPECT_THAT(llvm::toString(Results.takeError()), testing::HasSubstr("too many occurrences")); @@ -1305,6 +1317,7 @@ NewName, AST, MainFilePath, + getVFSFromAST(AST), &DIndex, {/*CrossFile=*/true}}); ASSERT_TRUE(bool(Results)) << Results.takeError(); @@ -1525,7 +1538,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)),