kbobyrev updated this revision to Diff 233037.
kbobyrev added a comment.
Improve wording in the comment I moved.
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
@@ -668,11 +668,85 @@
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 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(
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<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;
}
@@ -212,17 +234,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
