[PATCH] D135543: [clangd] Rename: Allow renaming virtual methods overriding multiple base methods [2/3]

[Tom Praschan via Phabricator via cfe-commits]

tom-anders created this revision.
tom-anders added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
tom-anders updated this revision to Diff 466392.
tom-anders added a comment.
tom-anders updated this revision to Diff 466394.
tom-anders published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

clang-format


tom-anders added a comment.

Fix test case


For virtual methods that override the same method from multiple base classes (e.g. diamond-shaped inheritance graph) previously only one of the base class methods would be renamed.
Now, we report a canonicalDeclaration for each base class and will thus correctly rename all of them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135543

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
@@ -1522,26 +1522,32 @@
           virtual void [[foo]]();
         };
         class Derived1 : public Base {
-          void [[f^oo]]() override;
+          void [[foo]]() override;
+        };
+        class Derived2 : public Base {
+          void [[foo]]() override;
         };
-        class NotDerived {
-          void foo() {};
+        class DerivedDerived : public Derived1, public Derived2 {
+          void [[f^oo]]() override;
         }
       )cpp",
           R"cpp(
         #include "foo.h"
         void Base::[[foo]]() {}
         void Derived1::[[foo]]() {}
+        void Derived2::[[foo]]() {}
+        void DerivedDerived::[[foo]]() {}
 
-        class Derived2 : public Derived1 {
+        class Derived3 : public Derived1 {
           void [[foo]]() override {};
         };
 
-        void func(Base* b, Derived1* d1, 
-                  Derived2* d2, NotDerived* nd) {
+        void func(Base* b, Derived1* d1, Derived2* d2,
+                  DerivedDerived* dd, NotDerived* nd) {
           b->[[foo]]();
           d1->[[foo]]();
           d2->[[foo]]();
+          dd->[[foo]]();
           nd->foo();
         }
       )cpp",
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -101,12 +101,14 @@
     if (const FunctionDecl *InstantiatedMethod =
             Method->getInstantiatedFromMemberFunction())
       return canonicalRenameDecls(InstantiatedMethod);
-    // FIXME(kirillbobyrev): For virtual methods with
-    // size_overridden_methods() > 1, this will not rename all functions it
-    // overrides, because this code assumes there is a single canonical
-    // declaration.
-    if (Method->isVirtual() && Method->size_overridden_methods())
-      return {canonicalRenameDecls(*Method->overridden_methods().begin())};
+    if (Method->isVirtual() && Method->size_overridden_methods()) {
+      llvm::DenseSet<const NamedDecl *> Result;
+      for (const auto *OM : Method->overridden_methods()) {
+        auto Decls = canonicalRenameDecls(OM);
+        Result.insert(Decls.begin(), Decls.end());
+      }
+      return Result;
+    }
   }
   if (const auto *Function = dyn_cast<FunctionDecl>(D))
     if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D135543.466394.patch
Type: text/x-patch
Size: 2630 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221009/611d3339/attachment.bin>