Author: Kirill Bobyrev Date: 2020-01-21T05:33:39+01:00 New Revision: 38bdb94120b76f8f79cd27d721892673e573895a
URL: https://github.com/llvm/llvm-project/commit/38bdb94120b76f8f79cd27d721892673e573895a DIFF: https://github.com/llvm/llvm-project/commit/38bdb94120b76f8f79cd27d721892673e573895a.diff LOG: [clangd] Fix rename for explicit destructor calls When triggering rename of the class name in the code with explicit destructor calls, rename fails. Consider the following piece of code: ``` class Foo; ... Foo f; f.~/*...*/Foo(); ``` `findExplicitReferences` will report two `ReferenceLoc` for destructor call: one is comming from `MemberExpr` (i.e. destructor call itself) and would point to the tilde: ``` f.~/*...*/Foo(); ^ ``` And the second one is pointing to the typename and is coming from `TypeLoc`. ``` f.~/*...*/Foo(); ^ ``` This causes rename to produce incorrect textual replacements. This patch updates `MemberExpr` handler to detect destructor calls and prevents it from reporting a duplicate reference. Resolves: https://github.com/clangd/clangd/issues/236 Reviewers: kadircet, hokein Differential Revision: https://reviews.llvm.org/D72638 Added: Modified: clang-tools-extra/clangd/FindTarget.cpp clang-tools-extra/clangd/unittests/FindTargetTests.cpp clang-tools-extra/clangd/unittests/RenameTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp index 2bab9d4f67b7..0e3c30e16dd5 100644 --- a/clang-tools-extra/clangd/FindTarget.cpp +++ b/clang-tools-extra/clangd/FindTarget.cpp @@ -614,6 +614,10 @@ llvm::SmallVector<ReferenceLoc, 2> refInExpr(const Expr *E) { } void VisitMemberExpr(const MemberExpr *E) { + // Skip destructor calls to avoid duplication: TypeLoc within will be + // visited separately. + if (llvm::dyn_cast<CXXDestructorDecl>(E->getFoundDecl().getDecl())) + return; Refs.push_back(ReferenceLoc{E->getQualifierLoc(), E->getMemberNameInfo().getLoc(), /*IsDecl=*/false, diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 408ebe24e773..a420348fcda8 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -879,6 +879,56 @@ TEST_F(FindExplicitReferencesTest, All) { "8: targets = {INT2}, decl\n" "9: targets = {NS}, decl\n" "10: targets = {ns}\n"}, + // User-defined conversion operator. + {R"cpp( + void foo() { + class $0^Bar {}; + class $1^Foo { + public: + // FIXME: This should have only one reference to Bar. + $2^operator $3^$4^Bar(); + }; + + $5^Foo $6^f; + $7^f.$8^operator $9^Bar(); + } + )cpp", + "0: targets = {Bar}, decl\n" + "1: targets = {Foo}, decl\n" + "2: targets = {foo()::Foo::operator Bar}, decl\n" + "3: targets = {Bar}\n" + "4: targets = {Bar}\n" + "5: targets = {Foo}\n" + "6: targets = {f}, decl\n" + "7: targets = {f}\n" + "8: targets = {foo()::Foo::operator Bar}\n" + "9: targets = {Bar}\n"}, + // Destructor. + {R"cpp( + void foo() { + class $0^Foo { + public: + ~$1^Foo() {} + + void $2^destructMe() { + this->~$3^Foo(); + } + }; + + $4^Foo $5^f; + $6^f.~ /*...*/ $7^Foo(); + } + )cpp", + "0: targets = {Foo}, decl\n" + // FIXME: It's better to target destructor's FunctionDecl instead of + // the type itself (similar to constructor). + "1: targets = {Foo}\n" + "2: targets = {foo()::Foo::destructMe}, decl\n" + "3: targets = {Foo}\n" + "4: targets = {Foo}\n" + "5: targets = {f}, decl\n" + "6: targets = {f}\n" + "7: targets = {Foo}\n"}, // cxx constructor initializer. {R"cpp( class Base {}; diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index 80930b9096d4..d0fc4d1033b5 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -265,6 +265,33 @@ TEST(RenameTest, WithinFileRename) { } )cpp", + // Destructor explicit call. + R"cpp( + class [[F^oo]] { + public: + ~[[^Foo]](); + }; + + [[Foo^]]::~[[^Foo]]() {} + + int main() { + [[Fo^o]] f; + f.~/*something*/[[^Foo]](); + f.~[[^Foo]](); + } + )cpp", + + // Derived destructor explicit call. + R"cpp( + class [[Bas^e]] {}; + class Derived : public [[Bas^e]] {} + + int main() { + [[Bas^e]] *foo = new Derived(); + foo->[[^Base]]::~[[^Base]](); + } + )cpp", + // CXXConstructor initializer list. R"cpp( class Baz {}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits