Author: Kirill Bobyrev Date: 2019-12-12T13:10:59+01:00 New Revision: ec618826dfb91c5413353ebcc54f360e43df10a0
URL: https://github.com/llvm/llvm-project/commit/ec618826dfb91c5413353ebcc54f360e43df10a0 DIFF: https://github.com/llvm/llvm-project/commit/ec618826dfb91c5413353ebcc54f360e43df10a0.diff LOG: [clangd] Rename constructors and destructors in cross-file case * Use ad-hoc Decl canonicalization from Clang-Rename to allow renaming constructors and destructors while using cross-file rename. * Manually handle the destructor selection * Add unit tests to prevent regressions and ensure the correct behaviour Reviewed by: sammccall Differential Revision: https://reviews.llvm.org/D71247 Added: Modified: clang-tools-extra/clangd/refactor/Rename.cpp clang-tools-extra/clangd/unittests/RenameTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index 8ca90afba69a..010f7e388b55 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -18,8 +18,10 @@ #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/Casting.h" #include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" #include <algorithm> @@ -83,21 +85,17 @@ llvm::DenseSet<const Decl *> locateDeclAt(ParsedAST &AST, 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)) { + const auto *ND = llvm::dyn_cast<NamedDecl>(D); + if (!ND) + continue; + // Get to CXXRecordDecl from constructor or destructor. + ND = tooling::getCanonicalSymbolDeclaration(ND); + Result.insert(ND); + } return Result; } @@ -214,17 +212,16 @@ llvm::Error makeError(ReasonToReject Reason) { // 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; + // 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. // 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()); + const auto RenameDecl = + ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND; + std::vector<std::string> RenameUSRs = + tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext()); llvm::DenseSet<SymbolID> TargetIDs; for (auto &USR : RenameUSRs) TargetIDs.insert(SymbolID(USR)); @@ -455,14 +452,21 @@ llvm::Expected<FileEdits> rename(const RenameInputs &RInputs) { 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) diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index f253625ed032..e98bf78323b9 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -487,11 +487,10 @@ TEST(RenameTest, Renameable) { "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() {} @@ -720,24 +719,24 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) { MockCompilationDatabase CDB; CDB.ExtraClangFlags = {"-xc++"}; class IgnoreDiagnostics : public DiagnosticsConsumer { - void onDiagnosticsReady(PathRef File, - std::vector<Diag> Diagnostics) override {} + void onDiagnosticsReady(PathRef File, + 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 [] = { - { - // classes. - R"cpp( + } Cases[] = { + { + // classes. + R"cpp( class [[Fo^o]] { [[Foo]](); ~[[Foo]](); }; )cpp", - R"cpp( + R"cpp( #include "foo.h" [[Foo]]::[[Foo]]() {} [[Foo]]::~[[Foo]]() {} @@ -746,15 +745,15 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) { [[Foo]] foo; } )cpp", - }, - { - // class methods. - R"cpp( + }, + { + // class methods. + R"cpp( class Foo { void [[f^oo]](); }; )cpp", - R"cpp( + R"cpp( #include "foo.h" void Foo::[[foo]]() {} @@ -762,13 +761,49 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) { p->[[foo]](); } )cpp", - }, - { - // functions. - R"cpp( + }, + { + // 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", + }, + { + // functions. + R"cpp( void [[f^oo]](); )cpp", - R"cpp( + R"cpp( #include "foo.h" void [[foo]]() {} @@ -776,63 +811,63 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) { [[foo]](); } )cpp", - }, - { - // typedefs. - R"cpp( + }, + { + // typedefs. + R"cpp( typedef int [[IN^T]]; [[INT]] foo(); )cpp", - R"cpp( + R"cpp( #include "foo.h" [[INT]] foo() {} )cpp", - }, - { - // usings. - R"cpp( + }, + { + // usings. + R"cpp( using [[I^NT]] = int; [[INT]] foo(); )cpp", - R"cpp( + R"cpp( #include "foo.h" [[INT]] foo() {} )cpp", - }, - { - // variables. - R"cpp( + }, + { + // variables. + R"cpp( static const int [[VA^R]] = 123; )cpp", - R"cpp( + R"cpp( #include "foo.h" int s = [[VAR]]; )cpp", - }, - { - // scope enums. - R"cpp( + }, + { + // scope enums. + R"cpp( enum class [[K^ind]] { ABC }; )cpp", - R"cpp( + R"cpp( #include "foo.h" [[Kind]] ff() { return [[Kind]]::ABC; } )cpp", - }, - { - // enum constants. - R"cpp( + }, + { + // enum constants. + R"cpp( enum class Kind { [[A^BC]] }; )cpp", - R"cpp( + R"cpp( #include "foo.h" Kind ff() { return Kind::[[ABC]]; } )cpp", - }, + }, }; for (const auto& T : Cases) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
