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 @@ -145,6 +145,8 @@ /// Enable cross-file rename feature. bool CrossFileRename = false; + unsigned GlobalRenameFileLimit = 50; + /// Returns true if the tweak should be enabled. std::function TweakFilter = [](const Tweak &T) { return !T.hidden(); // only enable non-hidden tweaks. @@ -295,7 +297,7 @@ /// Get all document links in a file. void documentLinks(PathRef File, Callback> CB); - + /// Returns estimated memory usage for each of the currently open files. /// The order of results is unspecified. /// Overall memory usage of clangd may be significantly more than reported @@ -341,6 +343,7 @@ bool SuggestMissingIncludes = false; bool CrossFileRename = false; + const unsigned GlobalRenameFileLimit = 50; std::function TweakFilter; 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 @@ -131,8 +131,9 @@ : nullptr), GetClangTidyOptions(Opts.GetClangTidyOptions), SuggestMissingIncludes(Opts.SuggestMissingIncludes), - CrossFileRename(Opts.CrossFileRename), TweakFilter(Opts.TweakFilter), - WorkspaceRoot(Opts.WorkspaceRoot), + CrossFileRename(Opts.CrossFileRename), + GlobalRenameFileLimit(Opts.GlobalRenameFileLimit), + TweakFilter(Opts.TweakFilter), WorkspaceRoot(Opts.WorkspaceRoot), // Pass a callback into `WorkScheduler` to extract symbols from a newly // parsed file and rebuild the file index synchronously each time an AST // is parsed. @@ -344,9 +345,10 @@ // Performing the local rename isn't substantially more expensive than // doing an AST-based check, so we just rename and throw away the results. - auto Changes = clangd::rename({Pos, "dummy", AST, File, Index, - /*AllowCrossFile=*/false, - /*GetDirtyBuffer=*/nullptr}); + auto Changes = + clangd::rename({Pos, "dummy", AST, File, Index, + /*AllowCrossFile=*/false, GlobalRenameFileLimit, + /*GetDirtyBuffer=*/nullptr}); if (!Changes) { // 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 @@ -374,8 +376,9 @@ return llvm::None; return It->second; }; - auto Edits = clangd::rename({Pos, NewName, InpAST->AST, File, Index, - CrossFileRename, GetDirtyBuffer}); + auto Edits = + clangd::rename({Pos, NewName, InpAST->AST, File, Index, CrossFileRename, + GlobalRenameFileLimit, GetDirtyBuffer}); if (!Edits) return CB(Edits.takeError()); 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 @@ -36,6 +36,7 @@ const SymbolIndex *Index = nullptr; bool AllowCrossFile = false; + const unsigned GlobalRenameFileLimit = 50; // 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 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 @@ -315,7 +315,8 @@ // grouped by the absolute file path. llvm::Expected>> findOccurrencesOutsideFile(const NamedDecl &RenameDecl, - llvm::StringRef MainFile, const SymbolIndex &Index) { + llvm::StringRef MainFile, const SymbolIndex &Index, + const unsigned GlobalRenameFileLimit) { trace::Span Tracer("FindOccurrencesOutsideFile"); RefsRequest RQuest; RQuest.IDs.insert(*getSymbolID(&RenameDecl)); @@ -330,10 +331,8 @@ // Absolute file path => rename occurrences in that file. llvm::StringMap> AffectedFiles; - // FIXME: Make the limit customizable. - static constexpr size_t MaxLimitFiles = 50; bool HasMore = Index.refs(RQuest, [&](const Ref &R) { - if (AffectedFiles.size() > MaxLimitFiles) + if (AffectedFiles.size() > GlobalRenameFileLimit) return; if ((R.Kind & RefKind::Spelled) == RefKind::Unknown) return; @@ -343,10 +342,10 @@ } }); - if (AffectedFiles.size() > MaxLimitFiles) + if (AffectedFiles.size() > GlobalRenameFileLimit) return llvm::make_error( llvm::formatv("The number of affected files exceeds the max limit {0}", - MaxLimitFiles), + GlobalRenameFileLimit), llvm::inconvertibleErrorCode()); if (HasMore) { return llvm::make_error( @@ -381,10 +380,11 @@ llvm::Expected renameOutsideFile( const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, llvm::StringRef NewName, const SymbolIndex &Index, - llvm::function_ref(PathRef)> GetFileContent) { + llvm::function_ref(PathRef)> GetFileContent, + const unsigned GlobalRenameFileLimit) { trace::Span Tracer("RenameOutsideFile"); - auto AffectedFiles = - findOccurrencesOutsideFile(RenameDecl, MainFilePath, Index); + auto AffectedFiles = findOccurrencesOutsideFile(RenameDecl, MainFilePath, + Index, GlobalRenameFileLimit); if (!AffectedFiles) return AffectedFiles.takeError(); FileEdits Results; @@ -541,9 +541,9 @@ // Renameable safely guards us that at this point we are renaming a local // symbol if we don't have index. if (RInputs.Index) { - auto OtherFilesEdits = - renameOutsideFile(RenameDecl, RInputs.MainFilePath, RInputs.NewName, - *RInputs.Index, GetFileContent); + auto OtherFilesEdits = renameOutsideFile( + RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index, + GetFileContent, RInputs.GlobalRenameFileLimit); if (!OtherFilesEdits) return OtherFilesEdits.takeError(); Results = std::move(*OtherFilesEdits); diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -402,6 +402,14 @@ init(false), }; +opt GlobalRenameFileLimit{ + "global-rename-file-limit", + cat(Features), + desc("Renaming symbol that appears in more files will fail"), + init(50), + Hidden, +}; + /// Supports a test URI scheme with relaxed constraints for lit tests. /// The path in a test URI will be combined with a platform-specific fake /// directory to form an absolute path. For example, test:///a.cpp is resolved @@ -601,6 +609,7 @@ } ClangdServer::Options Opts; + Opts.GlobalRenameFileLimit = GlobalRenameFileLimit; switch (PCHStorage) { case PCHStorageFlag::Memory: Opts.StorePreamblesInMemory = true; 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 @@ -715,7 +715,8 @@ auto AST = TU.build(); llvm::StringRef NewName = "newName"; auto Results = rename({MainCode.point(), NewName, AST, MainFilePath, - Index.get(), /*CrossFile=*/true, GetDirtyBuffer}); + Index.get(), /*CrossFile=*/true, + /*GlobalRenameFileLimit=*/50, GetDirtyBuffer}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(*Results)), @@ -731,7 +732,8 @@ TU.AdditionalFiles["bar.cc"] = std::string(BarCode.code()); AST = TU.build(); Results = rename({MainCode.point(), NewName, AST, MainFilePath, Index.get(), - /*CrossFile=*/true, GetDirtyBuffer}); + /*CrossFile=*/true, /*GlobalRenameFileLimit=*/50, + GetDirtyBuffer}); ASSERT_TRUE(bool(Results)) << Results.takeError(); EXPECT_THAT( applyEdits(std::move(*Results)), @@ -762,7 +764,8 @@ size_t estimateMemoryUsage() const override { return 0; } } PIndex; Results = rename({MainCode.point(), NewName, AST, MainFilePath, &PIndex, - /*CrossFile=*/true, GetDirtyBuffer}); + /*CrossFile=*/true, /*GlobalRenameFileLimit=*/50, + GetDirtyBuffer}); EXPECT_FALSE(Results); EXPECT_THAT(llvm::toString(Results.takeError()), testing::HasSubstr("too many occurrences"));