kbobyrev updated this revision to Diff 233533.
kbobyrev marked 5 inline comments as done.
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
@@ -487,11 +487,10 @@
"not a supported kind", HeaderFile, Index},
{
-
R"cpp(
struct X { X operator++(int); };
void f(X x) {x+^+;})cpp",
- "not a supported kind", HeaderFile, Index},
+ "no symbol", HeaderFile, Index},
{R"cpp(// foo is declared outside the file.
void fo^o() {}
@@ -728,7 +727,81 @@
struct Case {
llvm::StringRef FooH;
llvm::StringRef FooCC;
- } Cases [] = {
+ } 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 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
@@ -18,6 +18,7 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
+#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Error.h"
@@ -83,21 +84,20 @@
if (!SelectedNode)
return {};
- // If the location points to a Decl, we check it is actually on the name
- // 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 {};
- }
-
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 +214,8 @@
// 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));
@@ -455,14 +446,21 @@
return (*Content)->getBuffer().str();
};
- SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
- getBeginningOfIdentifier(RInputs.Pos, SM, AST.getLangOpts()));
+ // Try to find the tokens adjacent to the cursor position.
+ auto Loc = sourceLocationInMainFile(SM, RInputs.Pos);
+ if (!Loc)
+ return Loc.takeError();
+ const syntax::Token *IdentifierToken =
+ spelledIdentifierTouching(*Loc, AST.getTokens());
+ // Renames should only triggered on identifiers.
+ if (!IdentifierToken)
+ return makeError(ReasonToReject::NoSymbolFound);
// FIXME: Renaming macros is not supported yet, the macro-handling code should
// be moved to rename tooling library.
- if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor()))
+ if (locateMacroAt(IdentifierToken->location(), AST.getPreprocessor()))
return makeError(ReasonToReject::UnsupportedSymbol);
- auto DeclsUnderCursor = locateDeclAt(AST, SourceLocationBeg);
+ auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
if (DeclsUnderCursor.empty())
return makeError(ReasonToReject::NoSymbolFound);
if (DeclsUnderCursor.size() > 1)
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
