Index: clang-tools-extra/clangd/refactor/Rename.cpp =================================================================== --- clang-tools-extra/clangd/refactor/Rename.cpp +++ clang-tools-extra/clangd/refactor/Rename.cpp @@ -14,11 +14,14 @@ #include "Selection.h" #include "SourceCode.h" #include "index/SymbolCollector.h" +#include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclTemplate.h" #include "clang/Basic/SourceLocation.h" +#include "clang/Basic/TokenKinds.h" #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" @@ -85,17 +88,36 @@ // range of the Decl. This would avoid allowing rename on unrelated tokens. // ^class Foo {} // SelectionTree returns CXXRecordDecl, // // we don't attempt to trigger rename on this position. - // FIXME: Make this work on destructors, e.g. "~F^oo()". if (const auto *D = SelectedNode->ASTNode.get()) { - if (D->getLocation() != TokenStartLoc) - return {}; + if (D->getLocation() != TokenStartLoc) { + // Destructor->getLocation() points to ~. In this case, TokenStartLoc + // should point to the next token. + const auto *Destructor = llvm::dyn_cast(D); + if (!Destructor) + return {}; + // There should be exactly two tokens within inspected range: tok::tilde + // and tok::identifier. + const auto Tokens = AST.getTokens().expandedTokens( + {Destructor->getLocation(), TokenStartLoc}); + if (Tokens.size() != 2 || Tokens.back().kind() != tok::identifier) + return {}; + } } llvm::DenseSet Result; for (const auto *D : targetDecl(SelectedNode->ASTNode, - DeclRelation::Alias | DeclRelation::TemplatePattern)) - Result.insert(D); + DeclRelation::Alias | DeclRelation::TemplatePattern)) { + // If the cursor is under the underlying CXXRecordDecl of the + // ClassTemplateDecl, ND will be the CXXRecordDecl, for this case, we need + // to get the primary template maunally. + // FIXME: Do proper and well-defined canonicalization. + D = D->getDescribedTemplate() ? D->getDescribedTemplate() : D; + const auto *ND = llvm::dyn_cast(D); + // Get to CXXRecordDecl from constructor or destructor. + ND = tooling::getCanonicalSymbolDeclaration(ND); + Result.insert(ND); + } return Result; } @@ -212,17 +234,11 @@ // Return all rename occurrences in the main file. std::vector findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl &ND) { - // In theory, locateDeclAt should return the primary template. However, if the - // cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND - // will be the CXXRecordDecl, for this case, we need to get the primary - // template maunally. - const auto &RenameDecl = - ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND; // getUSRsForDeclaration will find other related symbols, e.g. virtual and its // overriddens, primary template and all explicit specializations. // FIXME: Get rid of the remaining tooling APIs. - std::vector RenameUSRs = tooling::getUSRsForDeclaration( - tooling::getCanonicalSymbolDeclaration(&RenameDecl), AST.getASTContext()); + std::vector RenameUSRs = + tooling::getUSRsForDeclaration(&ND, AST.getASTContext()); llvm::DenseSet TargetIDs; for (auto &USR : RenameUSRs) TargetIDs.insert(SymbolID(USR)); Index: clang-tools-extra/clangd/unittests/RenameTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/RenameTests.cpp +++ clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -668,11 +668,85 @@ std::vector Diagnostics) override {} } DiagConsumer; // rename is runnning on the "^" point in FooH, and "[[]]" ranges are the - // expcted rename occurrences. + // expected rename occurrences. struct Case { llvm::StringRef FooH; llvm::StringRef FooCC; } Cases [] = { + { + // Constructor. + R"cpp( + class [[Foo]] { + [[Foo^]](); + ~[[Foo]](); + }; + )cpp", + R"cpp( + #include "foo.h" + [[Foo]]::[[Foo]]() {} + [[Foo]]::~[[Foo]]() {} + + void func() { + [[Foo]] foo; + } + )cpp", + }, + { + // Destructor (selecting before the identifier). + R"cpp( + class [[Foo]] { + [[Foo]](); + ^~[[Foo]](); + }; + )cpp", + R"cpp( + #include "foo.h" + [[Foo]]::[[Foo]]() {} + [[Foo]]::~[[Foo]]() {} + + void func() { + [[Foo]] foo; + } + )cpp", + }, + { + // Destructor (selecting within the identifier). + R"cpp( + class [[Foo]] { + [[Foo]](); + ~[[F^oo]](); + }; + )cpp", + R"cpp( + #include "foo.h" + [[Foo]]::[[Foo]]() {} + [[Foo]]::~[[Foo]]() {} + + void func() { + [[Foo]] foo; + } + )cpp", + }, + { + // Destructor (selecting before the identifier). + R"cpp( + class [[Foo]] { + [[Foo]](); + virtual ~ /*~Foo?*/[[Foo^]]() { + int a = 4; + } + }; + )cpp", + R"cpp( + #include "foo.h" + [[Foo]]::[[Foo]]() {} + [[Foo]]::~[[Foo]]() {} + + void func() { + [[Foo]] foo; + } + )cpp", + }, { // classes. R"cpp(