hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74834
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/Rename.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -714,8 +714,13 @@
TestTU TU = TestTU::withCode(MainCode.code());
auto AST = TU.build();
llvm::StringRef NewName = "newName";
- auto Results = rename({MainCode.point(), NewName, AST, MainFilePath,
- Index.get(), /*CrossFile=*/true, GetDirtyBuffer});
+ auto Results = rename({MainCode.point(),
+ NewName,
+ AST,
+ MainFilePath,
+ Index.get(),
+ {/*CrossFile=*/true},
+ GetDirtyBuffer});
ASSERT_TRUE(bool(Results)) << Results.takeError();
EXPECT_THAT(
applyEdits(std::move(*Results)),
@@ -730,8 +735,13 @@
// 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(),
- /*CrossFile=*/true, GetDirtyBuffer});
+ Results = rename({MainCode.point(),
+ NewName,
+ AST,
+ MainFilePath,
+ Index.get(),
+ {/*CrossFile=*/true},
+ GetDirtyBuffer});
ASSERT_TRUE(bool(Results)) << Results.takeError();
EXPECT_THAT(
applyEdits(std::move(*Results)),
@@ -761,8 +771,13 @@
Callback) const override {}
size_t estimateMemoryUsage() const override { return 0; }
} PIndex;
- Results = rename({MainCode.point(), NewName, AST, MainFilePath, &PIndex,
- /*CrossFile=*/true, GetDirtyBuffer});
+ Results = rename({MainCode.point(),
+ NewName,
+ AST,
+ MainFilePath,
+ &PIndex,
+ {/*CrossFile=*/true},
+ GetDirtyBuffer});
EXPECT_FALSE(Results);
EXPECT_THAT(llvm::toString(Results.takeError()),
testing::HasSubstr("too many occurrences"));
@@ -803,9 +818,12 @@
RefsRequest *Out;
} RIndex(&Req);
- auto Results =
- rename({MainCode.point(), "NewName", AST, testPath("main.cc"), &RIndex,
- /*CrossFile=*/true});
+ auto Results = rename({MainCode.point(),
+ "NewName",
+ AST,
+ testPath("main.cc"),
+ &RIndex,
+ {/*CrossFile=*/true}});
ASSERT_TRUE(bool(Results)) << Results.takeError();
const auto HeaderSymbols = TU.headerSymbols();
EXPECT_THAT(Req.IDs,
@@ -850,8 +868,9 @@
Ref ReturnedRef;
} DIndex(XRefInBarCC);
llvm::StringRef NewName = "newName";
- auto Results = rename({MainCode.point(), NewName, AST, MainFilePath, &DIndex,
- /*CrossFile=*/true});
+ RenameOptions RenameOpts{true, 50};
+ auto Results = rename(
+ {MainCode.point(), NewName, AST, MainFilePath, &DIndex, RenameOpts});
ASSERT_TRUE(bool(Results)) << Results.takeError();
EXPECT_THAT(
applyEdits(std::move(*Results)),
@@ -1021,7 +1040,7 @@
FS.Files[FooCCPath] = std::string(FooCC.code());
auto ServerOpts = ClangdServer::optsForTest();
- ServerOpts.CrossFileRename = true;
+ ServerOpts.RenameOpts.AllowCrossFile = true;
ServerOpts.BuildDynamicSymbolIndex = true;
ClangdServer Server(CDB, FS, ServerOpts);
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -628,7 +628,7 @@
}
Opts.StaticIndex = StaticIdx.get();
Opts.AsyncThreadsCount = WorkerThreadsCount;
- Opts.CrossFileRename = CrossFileRename;
+ Opts.RenameOpts.AllowCrossFile = CrossFileRename;
clangd::CodeCompleteOptions CCOpts;
CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/refactor/Rename.h
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -26,6 +26,14 @@
using DirtyBufferGetter =
llvm::function_ref<llvm::Optional<std::string>(PathRef AbsPath)>;
+struct RenameOptions {
+ bool AllowCrossFile = false;
+ /// The mamimum number of affected files (0 means no limit), only meaningful
+ /// when AllowCrossFile = true.
+ /// If the actual number exceeds the limit, rename is forbidden.
+ size_t LimitFiles = 50;
+};
+
struct RenameInputs {
Position Pos; // the position triggering the rename
llvm::StringRef NewName;
@@ -35,7 +43,7 @@
const SymbolIndex *Index = nullptr;
- bool AllowCrossFile = false;
+ 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
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -322,7 +322,8 @@
// grouped by the absolute file path.
llvm::Expected<llvm::StringMap<std::vector<Range>>>
findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
- llvm::StringRef MainFile, const SymbolIndex &Index) {
+ llvm::StringRef MainFile, const SymbolIndex &Index,
+ size_t MaxLimitFiles) {
trace::Span Tracer("FindOccurrencesOutsideFile");
RefsRequest RQuest;
RQuest.IDs.insert(*getSymbolID(&RenameDecl));
@@ -337,8 +338,6 @@
// Absolute file path => rename occurrences in that file.
llvm::StringMap<std::vector<Range>> AffectedFiles;
- // FIXME: Make the limit customizable.
- static constexpr size_t MaxLimitFiles = 50;
bool HasMore = Index.refs(RQuest, [&](const Ref &R) {
if (AffectedFiles.size() > MaxLimitFiles)
return;
@@ -387,11 +386,11 @@
// there is no dirty buffer.
llvm::Expected<FileEdits> renameOutsideFile(
const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
- llvm::StringRef NewName, const SymbolIndex &Index,
+ llvm::StringRef NewName, const SymbolIndex &Index, size_t MaxLimitFiles,
llvm::function_ref<llvm::Expected<std::string>(PathRef)> GetFileContent) {
trace::Span Tracer("RenameOutsideFile");
- auto AffectedFiles =
- findOccurrencesOutsideFile(RenameDecl, MainFilePath, Index);
+ auto AffectedFiles = findOccurrencesOutsideFile(RenameDecl, MainFilePath,
+ Index, MaxLimitFiles);
if (!AffectedFiles)
return AffectedFiles.takeError();
FileEdits Results;
@@ -473,6 +472,7 @@
llvm::Expected<FileEdits> rename(const RenameInputs &RInputs) {
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());
@@ -520,7 +520,7 @@
const auto &RenameDecl =
llvm::cast<NamedDecl>(*(*DeclsUnderCursor.begin())->getCanonicalDecl());
auto Reject = renameable(RenameDecl, RInputs.MainFilePath, RInputs.Index,
- RInputs.AllowCrossFile);
+ Opts.AllowCrossFile);
if (Reject)
return makeError(*Reject);
@@ -537,7 +537,7 @@
if (!MainFileRenameEdit)
return MainFileRenameEdit.takeError();
- if (!RInputs.AllowCrossFile) {
+ if (!Opts.AllowCrossFile) {
// Within-file rename: just return the main file results.
return FileEdits(
{std::make_pair(RInputs.MainFilePath,
@@ -548,9 +548,11 @@
// 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,
+ Opts.LimitFiles == 0 ? std::numeric_limits<size_t>::max()
+ : Opts.LimitFiles - 1,
+ GetFileContent);
if (!OtherFilesEdits)
return OtherFilesEdits.takeError();
Results = std::move(*OtherFilesEdits);
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -142,8 +142,8 @@
/// Enable semantic highlighting features.
bool SemanticHighlighting = false;
- /// Enable cross-file rename feature.
- bool CrossFileRename = false;
+ /// Options for rename.
+ RenameOptions RenameOpts;
/// Returns true if the tweak should be enabled.
std::function<bool(const Tweak &)> TweakFilter = [](const Tweak &T) {
@@ -340,7 +340,7 @@
// can be caused by missing includes (e.g. member access in incomplete type).
bool SuggestMissingIncludes = false;
- bool CrossFileRename = false;
+ RenameOptions RenameOpts;
std::function<bool(const Tweak &)> TweakFilter;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -131,7 +131,7 @@
: nullptr),
GetClangTidyOptions(Opts.GetClangTidyOptions),
SuggestMissingIncludes(Opts.SuggestMissingIncludes),
- CrossFileRename(Opts.CrossFileRename), TweakFilter(Opts.TweakFilter),
+ RenameOpts(Opts.RenameOpts), 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
@@ -338,14 +338,13 @@
SM, CharSourceRange::getCharRange(TouchingIdentifier->location(),
TouchingIdentifier->endLocation()));
- if (CrossFileRename)
+ if (RenameOpts.AllowCrossFile)
// FIXME: we now assume cross-file rename always succeeds, revisit this.
return CB(Range);
// 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,
+ auto Changes = clangd::rename({Pos, "dummy", AST, File, Index, RenameOpts,
/*GetDirtyBuffer=*/nullptr});
if (!Changes) {
// LSP says to return null on failure, but that will result in a generic
@@ -374,8 +373,8 @@
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, RenameOpts, GetDirtyBuffer});
if (!Edits)
return CB(Edits.takeError());
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
