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())
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())
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits