hokein created this revision.
hokein added reviewers: arphaman, kadircet.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.
SymbolOccurrences is not guaranteed to be unique -- as some parts of the
AST may get traversed twice, we may add duplicated occurrences, we
should deduplicate them.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D60827
Files:
clang-tools-extra/unittests/clangd/ClangdTests.cpp
clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
Index: clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===================================================================
--- clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -79,8 +79,8 @@
// Non-visitors:
- /// Returns a set of unique symbol occurrences. Duplicate or
- /// overlapping occurrences are erroneous and should be reported!
+ /// Returns a set of unique symbol occurrences. Overlapping occurrences are
+ /// erroneous and should be reported!
SymbolOccurrences takeOccurrences() { return std::move(Occurrences); }
private:
@@ -95,9 +95,20 @@
// The token of the source location we find actually has the old
// name.
- if (Offset != StringRef::npos)
- Occurrences.emplace_back(PrevName, SymbolOccurrence::MatchingSymbol,
- BeginLoc.getLocWithOffset(Offset));
+ if (Offset != StringRef::npos) {
+ SymbolOccurrence NewOccurrence = {PrevName,
+ SymbolOccurrence::MatchingSymbol,
+ BeginLoc.getLocWithOffset(Offset)};
+ // Don't insert duplicated occurrences.
+ auto It = llvm::find_if(
+ Occurrences, [&NewOccurrence](const SymbolOccurrence &L) {
+ return std::make_pair(L.getKind(), L.getNameRanges().vec()) ==
+ std::make_pair(NewOccurrence.getKind(),
+ NewOccurrence.getNameRanges().vec());
+ });
+ if (It == Occurrences.end())
+ Occurrences.push_back(std::move(NewOccurrence));
+ }
}
const std::set<std::string> USRSet;
Index: clang-tools-extra/unittests/clangd/ClangdTests.cpp
===================================================================
--- clang-tools-extra/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/unittests/clangd/ClangdTests.cpp
@@ -1157,6 +1157,33 @@
Field(&CodeCompletion::Scope, "ns::"))));
}
+TEST_F(ClangdVFSTest, NoDuplicatedTextEditsOnRename) {
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ auto FooCpp = testPath("foo.cpp");
+ Annotations Code(R"cpp(
+ struct [[Foo]] {};
+ [[Foo]] test() {
+ [[F^oo]] x;
+ return x;
+ }
+ )cpp");
+
+ runAddDocument(Server, FooCpp, Code.code());
+
+ auto TextEdits =
+ llvm::cantFail(runRename(Server, FooCpp, Code.point(), "new_name"));
+ std::vector<Range> ReplacementRanges;
+ for (const auto& TestEdit : TextEdits)
+ ReplacementRanges.push_back(TestEdit.range);
+ EXPECT_THAT(ReplacementRanges,
+ testing::UnorderedElementsAreArray(Code.ranges()));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===================================================================
--- clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -79,8 +79,8 @@
// Non-visitors:
- /// Returns a set of unique symbol occurrences. Duplicate or
- /// overlapping occurrences are erroneous and should be reported!
+ /// Returns a set of unique symbol occurrences. Overlapping occurrences are
+ /// erroneous and should be reported!
SymbolOccurrences takeOccurrences() { return std::move(Occurrences); }
private:
@@ -95,9 +95,20 @@
// The token of the source location we find actually has the old
// name.
- if (Offset != StringRef::npos)
- Occurrences.emplace_back(PrevName, SymbolOccurrence::MatchingSymbol,
- BeginLoc.getLocWithOffset(Offset));
+ if (Offset != StringRef::npos) {
+ SymbolOccurrence NewOccurrence = {PrevName,
+ SymbolOccurrence::MatchingSymbol,
+ BeginLoc.getLocWithOffset(Offset)};
+ // Don't insert duplicated occurrences.
+ auto It = llvm::find_if(
+ Occurrences, [&NewOccurrence](const SymbolOccurrence &L) {
+ return std::make_pair(L.getKind(), L.getNameRanges().vec()) ==
+ std::make_pair(NewOccurrence.getKind(),
+ NewOccurrence.getNameRanges().vec());
+ });
+ if (It == Occurrences.end())
+ Occurrences.push_back(std::move(NewOccurrence));
+ }
}
const std::set<std::string> USRSet;
Index: clang-tools-extra/unittests/clangd/ClangdTests.cpp
===================================================================
--- clang-tools-extra/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/unittests/clangd/ClangdTests.cpp
@@ -1157,6 +1157,33 @@
Field(&CodeCompletion::Scope, "ns::"))));
}
+TEST_F(ClangdVFSTest, NoDuplicatedTextEditsOnRename) {
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ auto FooCpp = testPath("foo.cpp");
+ Annotations Code(R"cpp(
+ struct [[Foo]] {};
+ [[Foo]] test() {
+ [[F^oo]] x;
+ return x;
+ }
+ )cpp");
+
+ runAddDocument(Server, FooCpp, Code.code());
+
+ auto TextEdits =
+ llvm::cantFail(runRename(Server, FooCpp, Code.point(), "new_name"));
+ std::vector<Range> ReplacementRanges;
+ for (const auto& TestEdit : TextEdits)
+ ReplacementRanges.push_back(TestEdit.range);
+ EXPECT_THAT(ReplacementRanges,
+ testing::UnorderedElementsAreArray(Code.ranges()));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
