tom-anders updated this revision to Diff 458221.
tom-anders added a comment.
Add test case that triggers assertion, limit recursion
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132797/new/
https://reviews.llvm.org/D132797
Files:
clang-tools-extra/clangd/refactor/Rename.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
@@ -886,12 +886,6 @@
@end
)cpp",
"not a supported kind", HeaderFile},
- {R"cpp(// FIXME: rename virtual/override methods is not supported yet.
- struct A {
- virtual void f^oo() {}
- };
- )cpp",
- "not a supported kind", !HeaderFile},
{R"cpp(
void foo(int);
void foo(char);
@@ -1490,6 +1484,44 @@
}
)cpp",
},
+ {
+ // virtual methods.
+ R"cpp(
+ class Base {
+ virtual void [[foo]]();
+ };
+ class Derived1 : public Base {
+ void [[f^oo]]() override;
+ };
+ class NotDerived {
+ void foo() {};
+ }
+ )cpp",
+ R"cpp(
+ #include "foo.h"
+ void Base::[[foo]]() {}
+ void Derived1::[[foo]]() {}
+
+ class Derived2 : public Derived1 {
+ void [[foo]]() override {};
+ };
+
+ void func(Base* b, Derived1* d1,
+ Derived2* d2, NotDerived* nd) {
+ b->[[foo]]();
+ d1->[[foo]]();
+ d2->[[foo]]();
+ nd->foo();
+ }
+ )cpp",
+ },
+ {
+ // virtual templated method
+ R"cpp(
+ template <typename> class Foo { virtual void [[m]](); };
+ class Bar : Foo<int> { void [[^m]]() override; };
+ )cpp", ""
+ },
{
// rename on constructor and destructor.
R"cpp(
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -214,13 +214,6 @@
IsMainFileOnly))
return ReasonToReject::NonIndexable;
-
- // FIXME: Renaming virtual methods requires to rename all overridens in
- // subclasses, our index doesn't have this information.
- if (const auto *S = llvm::dyn_cast<CXXMethodDecl>(&RenameDecl)) {
- if (S->isVirtual())
- return ReasonToReject::UnsupportedSymbol;
- }
return None;
}
@@ -551,6 +544,35 @@
return R;
}
+// Walk down from a virtual method to overriding methods, we rename them as a
+// group. Note that canonicalRenameDecl() ensures we're starting from the base
+// method.
+void recursivelyInsertOverrides(SymbolID Base, llvm::DenseSet<SymbolID> &IDs,
+ const SymbolIndex &Index,
+ llvm::DenseSet<SymbolID> &Pending) {
+ // By keeping track of symbols we've already visited (Pending), the number of
+ // requests is bounded by the length of the shortest chain to the most
+ // distance SymbolID
+ while (!Pending.empty()) {
+ RelationsRequest Req;
+ Req.Predicate = RelationKind::OverriddenBy;
+ Req.Subjects = Pending;
+ Pending.clear();
+
+ Index.relations(Req, [&](const SymbolID &, const Symbol &Override) {
+ IDs.insert(Override.ID);
+ Pending.insert(Override.ID);
+ recursivelyInsertOverrides(Override.ID, IDs, Index, Pending);
+ });
+ }
+}
+
+void recursivelyInsertOverrides(SymbolID Base, llvm::DenseSet<SymbolID> &IDs,
+ const SymbolIndex &Index) {
+ llvm::DenseSet<SymbolID> Pending = {Base};
+ recursivelyInsertOverrides(Base, IDs, Index, Pending);
+}
+
// Return all rename occurrences (using the index) outside of the main file,
// grouped by the absolute file path.
llvm::Expected<llvm::StringMap<std::vector<Range>>>
@@ -561,6 +583,10 @@
RefsRequest RQuest;
RQuest.IDs.insert(getSymbolID(&RenameDecl));
+ if (const auto *MethodDecl = llvm::dyn_cast<CXXMethodDecl>(&RenameDecl))
+ if (MethodDecl->isVirtual())
+ recursivelyInsertOverrides(*RQuest.IDs.begin(), RQuest.IDs, Index);
+
// Absolute file path => rename occurrences in that file.
llvm::StringMap<std::vector<Range>> AffectedFiles;
bool HasMore = Index.refs(RQuest, [&](const Ref &R) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
