kbobyrev updated this revision to Diff 233048. kbobyrev marked an inline comment as done. kbobyrev added a comment.
Add another test for `~^Foo` and rebase on top of master. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71247/new/ https://reviews.llvm.org/D71247 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 @@ -672,11 +672,103 @@ std::vector<Diag> 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 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 after 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( Index: clang-tools-extra/clangd/refactor/Rename.cpp =================================================================== --- clang-tools-extra/clangd/refactor/Rename.cpp +++ clang-tools-extra/clangd/refactor/Rename.cpp @@ -87,17 +87,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<Decl>()) { - 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<CXXDestructorDecl>(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<const Decl *> Result; for (const auto *D : targetDecl(SelectedNode->ASTNode, - DeclRelation::Alias | DeclRelation::TemplatePattern)) - Result.insert(D); + DeclRelation::Alias | DeclRelation::TemplatePattern)) { + // If the cursor is at the underlying CXXRecordDecl of the + // ClassTemplateDecl, ND will be the CXXRecordDecl. In 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<NamedDecl>(D); + // Get to CXXRecordDecl from constructor or destructor. + ND = tooling::getCanonicalSymbolDeclaration(ND); + Result.insert(ND); + } return Result; } @@ -214,17 +233,11 @@ // Return all rename occurrences in the main file. std::vector<SourceLocation> 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<std::string> RenameUSRs = tooling::getUSRsForDeclaration( - tooling::getCanonicalSymbolDeclaration(&RenameDecl), AST.getASTContext()); + std::vector<std::string> RenameUSRs = + tooling::getUSRsForDeclaration(&ND, AST.getASTContext()); llvm::DenseSet<SymbolID> TargetIDs; for (auto &USR : RenameUSRs) TargetIDs.insert(SymbolID(USR));
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits