From: Alex Lorenz Date: Thu, 13 Jul 2017 10:36:33 +0000 (+0000) Subject: [refactor][rename] Use a single base class for class that finds X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ffd70202d375d7ff98509a2ecd3c8318dd692fc9;p=clang [refactor][rename] Use a single base class for class that finds a declaration at location and for class that searches for all occurrences of a specific declaration This commit uses a single RecursiveSymbolVisitor class for both USRLocFindingASTVisitor and NamedDeclOccurrenceFindingVisitor to avoid duplicate traversal code. It also traverses nested name specifier locs in the new class and remove the separate matching step. Differential Revision: https://reviews.llvm.org/D34949 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@307898 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h b/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h new file mode 100644 index 0000000000..5fe1ff7673 --- /dev/null +++ b/include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h @@ -0,0 +1,124 @@ +//===--- RecursiveSymbolVisitor.h - Clang refactoring library -------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// \brief A wrapper class around \c RecursiveASTVisitor that visits each +/// occurrences of a named symbol. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLING_REFACTOR_RECURSIVE_SYMBOL_VISITOR_H +#define LLVM_CLANG_TOOLING_REFACTOR_RECURSIVE_SYMBOL_VISITOR_H + +#include "clang/AST/AST.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Lex/Lexer.h" + +using namespace llvm; + +namespace clang { +namespace tooling { + +/// Traverses the AST and visits the occurrence of each named symbol in the +/// given nodes. +template +class RecursiveSymbolVisitor + : public RecursiveASTVisitor> { + using BaseType = RecursiveASTVisitor>; + +public: + RecursiveSymbolVisitor(const SourceManager &SM, const LangOptions &LangOpts) + : SM(SM), LangOpts(LangOpts) {} + + bool visitSymbolOccurrence(const NamedDecl *ND, + ArrayRef NameRanges) { + return true; + } + + // Declaration visitors: + + bool VisitNamedDecl(const NamedDecl *D) { + return isa(D) ? true : visit(D, D->getLocation()); + } + + bool VisitCXXConstructorDecl(const CXXConstructorDecl *CD) { + for (const auto *Initializer : CD->inits()) { + // Ignore implicit initializers. + if (!Initializer->isWritten()) + continue; + if (const FieldDecl *FD = Initializer->getMember()) { + if (!visit(FD, Initializer->getSourceLocation(), + Lexer::getLocForEndOfToken(Initializer->getSourceLocation(), + 0, SM, LangOpts))) + return false; + } + } + return true; + } + + // Expression visitors: + + bool VisitDeclRefExpr(const DeclRefExpr *Expr) { + return visit(Expr->getFoundDecl(), Expr->getLocation()); + } + + bool VisitMemberExpr(const MemberExpr *Expr) { + return visit(Expr->getFoundDecl().getDecl(), Expr->getMemberLoc()); + } + + // Other visitors: + + bool VisitTypeLoc(const TypeLoc Loc) { + const SourceLocation TypeBeginLoc = Loc.getBeginLoc(); + const SourceLocation TypeEndLoc = + Lexer::getLocForEndOfToken(TypeBeginLoc, 0, SM, LangOpts); + if (const auto *TemplateTypeParm = + dyn_cast(Loc.getType())) { + if (!visit(TemplateTypeParm->getDecl(), TypeBeginLoc, TypeEndLoc)) + return false; + } + if (const auto *TemplateSpecType = + dyn_cast(Loc.getType())) { + if (!visit(TemplateSpecType->getTemplateName().getAsTemplateDecl(), + TypeBeginLoc, TypeEndLoc)) + return false; + } + return visit(Loc.getType()->getAsCXXRecordDecl(), TypeBeginLoc, TypeEndLoc); + } + + bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) { + // The base visitor will visit NNSL prefixes, so we should only look at + // the current NNS. + if (NNS) { + const NamespaceDecl *ND = NNS.getNestedNameSpecifier()->getAsNamespace(); + if (!visit(ND, NNS.getLocalBeginLoc(), NNS.getLocalEndLoc())) + return false; + } + return BaseType::TraverseNestedNameSpecifierLoc(NNS); + } + +private: + const SourceManager &SM; + const LangOptions &LangOpts; + + bool visit(const NamedDecl *ND, SourceLocation BeginLoc, + SourceLocation EndLoc) { + return static_cast(this)->visitSymbolOccurrence( + ND, SourceRange(BeginLoc, EndLoc)); + } + bool visit(const NamedDecl *ND, SourceLocation Loc) { + return visit(ND, Loc, + Loc.getLocWithOffset(ND->getNameAsString().length() - 1)); + } +}; + +} // end namespace tooling +} // end namespace clang + +#endif // LLVM_CLANG_TOOLING_REFACTOR_RECURSIVE_SYMBOL_VISITOR_H diff --git a/include/clang/Tooling/Refactoring/Rename/USRFinder.h b/include/clang/Tooling/Refactoring/Rename/USRFinder.h index 28d541af43..356243e5b1 100644 --- a/include/clang/Tooling/Refactoring/Rename/USRFinder.h +++ b/include/clang/Tooling/Refactoring/Rename/USRFinder.h @@ -18,12 +18,10 @@ #include "clang/AST/AST.h" #include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" #include #include using namespace llvm; -using namespace clang::ast_matchers; namespace clang { @@ -48,36 +46,6 @@ const NamedDecl *getNamedDeclFor(const ASTContext &Context, // Converts a Decl into a USR. std::string getUSRForDecl(const Decl *Decl); -// FIXME: Implement RecursiveASTVisitor::VisitNestedNameSpecifier instead. -class NestedNameSpecifierLocFinder : public MatchFinder::MatchCallback { -public: - explicit NestedNameSpecifierLocFinder(ASTContext &Context) - : Context(Context) {} - - std::vector getNestedNameSpecifierLocations() { - addMatchers(); - Finder.matchAST(Context); - return Locations; - } - -private: - void addMatchers() { - const auto NestedNameSpecifierLocMatcher = - nestedNameSpecifierLoc().bind("nestedNameSpecifierLoc"); - Finder.addMatcher(NestedNameSpecifierLocMatcher, this); - } - - void run(const MatchFinder::MatchResult &Result) override { - const auto *NNS = Result.Nodes.getNodeAs( - "nestedNameSpecifierLoc"); - Locations.push_back(*NNS); - } - - ASTContext &Context; - std::vector Locations; - MatchFinder Finder; -}; - } // end namespace tooling } // end namespace clang diff --git a/include/clang/module.modulemap b/include/clang/module.modulemap index f7e338d933..d850bd552e 100644 --- a/include/clang/module.modulemap +++ b/include/clang/module.modulemap @@ -138,5 +138,4 @@ module Clang_Tooling { // importing the AST matchers library gives a link dependency on the AST // matchers (and thus the AST), which clang-format should not have. exclude header "Tooling/RefactoringCallbacks.h" - exclude header "Tooling/Refactoring/Rename/USRFinder.h" } diff --git a/lib/Tooling/Refactoring/Rename/USRFinder.cpp b/lib/Tooling/Refactoring/Rename/USRFinder.cpp index f36387dafd..3bfb5bbe35 100644 --- a/lib/Tooling/Refactoring/Rename/USRFinder.cpp +++ b/lib/Tooling/Refactoring/Rename/USRFinder.cpp @@ -18,6 +18,7 @@ #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Index/USRGeneration.h" #include "clang/Lex/Lexer.h" +#include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h" #include "llvm/ADT/SmallVector.h" using namespace llvm; @@ -25,132 +26,38 @@ using namespace llvm; namespace clang { namespace tooling { -// NamedDeclFindingASTVisitor recursively visits each AST node to find the -// symbol underneath the cursor. -// FIXME: move to separate .h/.cc file if this gets too large. namespace { -class NamedDeclFindingASTVisitor - : public clang::RecursiveASTVisitor { + +/// Recursively visits each AST node to find the symbol underneath the cursor. +class NamedDeclOccurrenceFindingVisitor + : public RecursiveSymbolVisitor { public: // \brief Finds the NamedDecl at a point in the source. // \param Point the location in the source to search for the NamedDecl. - explicit NamedDeclFindingASTVisitor(const SourceLocation Point, - const ASTContext &Context) - : Result(nullptr), Point(Point), Context(Context) {} - - // \brief Finds the NamedDecl for a name in the source. - // \param Name the fully qualified name. - explicit NamedDeclFindingASTVisitor(const std::string &Name, - const ASTContext &Context) - : Result(nullptr), Name(Name), Context(Context) {} - - // Declaration visitors: - - // \brief Checks if the point falls within the NameDecl. This covers every - // declaration of a named entity that we may come across. Usually, just - // checking if the point lies within the length of the name of the declaration - // and the start location is sufficient. - bool VisitNamedDecl(const NamedDecl *Decl) { - return dyn_cast(Decl) - ? true - : setResult(Decl, Decl->getLocation(), - Decl->getNameAsString().length()); - } - - // Expression visitors: - - bool VisitDeclRefExpr(const DeclRefExpr *Expr) { - const NamedDecl *Decl = Expr->getFoundDecl(); - return setResult(Decl, Expr->getLocation(), - Decl->getNameAsString().length()); - } - - bool VisitMemberExpr(const MemberExpr *Expr) { - const NamedDecl *Decl = Expr->getFoundDecl().getDecl(); - return setResult(Decl, Expr->getMemberLoc(), - Decl->getNameAsString().length()); - } - - // Other visitors: - - bool VisitTypeLoc(const TypeLoc Loc) { - const SourceLocation TypeBeginLoc = Loc.getBeginLoc(); - const SourceLocation TypeEndLoc = Lexer::getLocForEndOfToken( - TypeBeginLoc, 0, Context.getSourceManager(), Context.getLangOpts()); - if (const auto *TemplateTypeParm = - dyn_cast(Loc.getType())) - return setResult(TemplateTypeParm->getDecl(), TypeBeginLoc, TypeEndLoc); - if (const auto *TemplateSpecType = - dyn_cast(Loc.getType())) { - return setResult(TemplateSpecType->getTemplateName().getAsTemplateDecl(), - TypeBeginLoc, TypeEndLoc); - } - return setResult(Loc.getType()->getAsCXXRecordDecl(), TypeBeginLoc, - TypeEndLoc); - } - - bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) { - for (const auto *Initializer : ConstructorDecl->inits()) { - // Ignore implicit initializers. - if (!Initializer->isWritten()) - continue; - if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) { - const SourceLocation InitBeginLoc = Initializer->getSourceLocation(), - InitEndLoc = Lexer::getLocForEndOfToken( - InitBeginLoc, 0, Context.getSourceManager(), - Context.getLangOpts()); - if (!setResult(FieldDecl, InitBeginLoc, InitEndLoc)) - return false; - } - } - return true; - } - - // Other: - - const NamedDecl *getNamedDecl() { return Result; } - - // \brief Determines if a namespace qualifier contains the point. - // \returns false on success and sets Result. - void handleNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) { - while (NameLoc) { - const NamespaceDecl *Decl = - NameLoc.getNestedNameSpecifier()->getAsNamespace(); - setResult(Decl, NameLoc.getLocalBeginLoc(), NameLoc.getLocalEndLoc()); - NameLoc = NameLoc.getPrefix(); - } - } - -private: - // \brief Sets Result to Decl if the Point is within Start and End. - // \returns false on success. - bool setResult(const NamedDecl *Decl, SourceLocation Start, - SourceLocation End) { - if (!Decl) + explicit NamedDeclOccurrenceFindingVisitor(const SourceLocation Point, + const ASTContext &Context) + : RecursiveSymbolVisitor(Context.getSourceManager(), + Context.getLangOpts()), + Point(Point), Context(Context) {} + + bool visitSymbolOccurrence(const NamedDecl *ND, + ArrayRef NameRanges) { + if (!ND) return true; - if (Name.empty()) { - // Offset is used to find the declaration. + for (const auto &Range : NameRanges) { + SourceLocation Start = Range.getBegin(); + SourceLocation End = Range.getEnd(); if (!Start.isValid() || !Start.isFileID() || !End.isValid() || !End.isFileID() || !isPointWithin(Start, End)) return true; - } else { - // Fully qualified name is used to find the declaration. - if (Name != Decl->getQualifiedNameAsString() && - Name != "::" + Decl->getQualifiedNameAsString()) - return true; } - Result = Decl; + Result = ND; return false; } - // \brief Sets Result to Decl if Point is within Loc and Loc + Offset. - // \returns false on success. - bool setResult(const NamedDecl *Decl, SourceLocation Loc, unsigned Offset) { - // FIXME: Add test for Offset == 0. Add test for Offset - 1 (vs -2 etc). - return Offset == 0 || - setResult(Decl, Loc, Loc.getLocWithOffset(Offset - 1)); - } + const NamedDecl *getNamedDecl() const { return Result; } +private: // \brief Determines if the Point is within Start and End. bool isPointWithin(const SourceLocation Start, const SourceLocation End) { // FIXME: Add tests for Point == End. @@ -160,17 +67,17 @@ private: Context.getSourceManager().isBeforeInTranslationUnit(Point, End)); } - const NamedDecl *Result; + const NamedDecl *Result = nullptr; const SourceLocation Point; // The location to find the NamedDecl. - const std::string Name; const ASTContext &Context; }; -} // namespace + +} // end anonymous namespace const NamedDecl *getNamedDeclAt(const ASTContext &Context, const SourceLocation Point) { const SourceManager &SM = Context.getSourceManager(); - NamedDeclFindingASTVisitor Visitor(Point, Context); + NamedDeclOccurrenceFindingVisitor Visitor(Point, Context); // Try to be clever about pruning down the number of top-level declarations we // see. If both start and end is either before or after the point we're @@ -184,18 +91,44 @@ const NamedDecl *getNamedDeclAt(const ASTContext &Context, Visitor.TraverseDecl(CurrDecl); } - NestedNameSpecifierLocFinder Finder(const_cast(Context)); - for (const auto &Location : Finder.getNestedNameSpecifierLocations()) - Visitor.handleNestedNameSpecifierLoc(Location); - return Visitor.getNamedDecl(); } +namespace { + +/// Recursively visits each NamedDecl node to find the declaration with a +/// specific name. +class NamedDeclFindingVisitor + : public RecursiveASTVisitor { +public: + explicit NamedDeclFindingVisitor(StringRef Name) : Name(Name) {} + + // We don't have to traverse the uses to find some declaration with a + // specific name, so just visit the named declarations. + bool VisitNamedDecl(const NamedDecl *ND) { + if (!ND) + return true; + // Fully qualified name is used to find the declaration. + if (Name != ND->getQualifiedNameAsString() && + Name != "::" + ND->getQualifiedNameAsString()) + return true; + Result = ND; + return false; + } + + const NamedDecl *getNamedDecl() const { return Result; } + +private: + const NamedDecl *Result = nullptr; + StringRef Name; +}; + +} // end anonymous namespace + const NamedDecl *getNamedDeclFor(const ASTContext &Context, const std::string &Name) { - NamedDeclFindingASTVisitor Visitor(Name, Context); + NamedDeclFindingVisitor Visitor(Name); Visitor.TraverseDecl(Context.getTranslationUnitDecl()); - return Visitor.getNamedDecl(); } diff --git a/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp b/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp index 934507fe6e..dc21a94610 100644 --- a/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp +++ b/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp @@ -22,6 +22,7 @@ #include "clang/Basic/SourceManager.h" #include "clang/Lex/Lexer.h" #include "clang/Tooling/Core/Lookup.h" +#include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h" #include "clang/Tooling/Refactoring/Rename/USRFinder.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" @@ -40,70 +41,27 @@ namespace { // \brief This visitor recursively searches for all instances of a USR in a // translation unit and stores them for later usage. class USRLocFindingASTVisitor - : public clang::RecursiveASTVisitor { + : public RecursiveSymbolVisitor { public: explicit USRLocFindingASTVisitor(const std::vector &USRs, StringRef PrevName, const ASTContext &Context) - : USRSet(USRs.begin(), USRs.end()), PrevName(PrevName), Context(Context) { + : RecursiveSymbolVisitor(Context.getSourceManager(), + Context.getLangOpts()), + USRSet(USRs.begin(), USRs.end()), PrevName(PrevName), Context(Context) { } - // Declaration visitors: - - bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) { - for (const auto *Initializer : ConstructorDecl->inits()) { - // Ignore implicit initializers. - if (!Initializer->isWritten()) - continue; - if (const clang::FieldDecl *FieldDecl = Initializer->getMember()) { - if (USRSet.find(getUSRForDecl(FieldDecl)) != USRSet.end()) - LocationsFound.push_back(Initializer->getSourceLocation()); - } - } - return true; - } - - bool VisitNamedDecl(const NamedDecl *Decl) { - if (USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) - checkAndAddLocation(Decl->getLocation()); - return true; - } - - // Expression visitors: - - bool VisitDeclRefExpr(const DeclRefExpr *Expr) { - const NamedDecl *Decl = Expr->getFoundDecl(); - - if (USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) { - const SourceManager &Manager = Decl->getASTContext().getSourceManager(); - SourceLocation Location = Manager.getSpellingLoc(Expr->getLocation()); - checkAndAddLocation(Location); - } - - return true; - } - - bool VisitMemberExpr(const MemberExpr *Expr) { - const NamedDecl *Decl = Expr->getFoundDecl().getDecl(); - if (USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) { - const SourceManager &Manager = Decl->getASTContext().getSourceManager(); - SourceLocation Location = Manager.getSpellingLoc(Expr->getMemberLoc()); - checkAndAddLocation(Location); - } - return true; - } - - // Other visitors: - - bool VisitTypeLoc(const TypeLoc Loc) { - if (USRSet.find(getUSRForDecl(Loc.getType()->getAsCXXRecordDecl())) != - USRSet.end()) - checkAndAddLocation(Loc.getBeginLoc()); - if (const auto *TemplateTypeParm = - dyn_cast(Loc.getType())) { - if (USRSet.find(getUSRForDecl(TemplateTypeParm->getDecl())) != - USRSet.end()) - checkAndAddLocation(Loc.getBeginLoc()); + bool visitSymbolOccurrence(const NamedDecl *ND, + ArrayRef NameRanges) { + if (USRSet.find(getUSRForDecl(ND)) != USRSet.end()) { + assert(NameRanges.size() == 1 && + "Multiple name pieces are not supported yet!"); + SourceLocation Loc = NameRanges[0].getBegin(); + const SourceManager &SM = Context.getSourceManager(); + // TODO: Deal with macro occurrences correctly. + if (Loc.isMacroID()) + Loc = SM.getSpellingLoc(Loc); + checkAndAddLocation(Loc); } return true; } @@ -116,17 +74,6 @@ public: return LocationsFound; } - // Namespace traversal: - void handleNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) { - while (NameLoc) { - const NamespaceDecl *Decl = - NameLoc.getNestedNameSpecifier()->getAsNamespace(); - if (Decl && USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) - checkAndAddLocation(NameLoc.getLocalBeginLoc()); - NameLoc = NameLoc.getPrefix(); - } - } - private: void checkAndAddLocation(SourceLocation Loc) { const SourceLocation BeginLoc = Loc; @@ -449,11 +396,6 @@ getLocationsOfUSRs(const std::vector &USRs, StringRef PrevName, Decl *Decl) { USRLocFindingASTVisitor Visitor(USRs, PrevName, Decl->getASTContext()); Visitor.TraverseDecl(Decl); - NestedNameSpecifierLocFinder Finder(Decl->getASTContext()); - - for (const auto &Location : Finder.getNestedNameSpecifierLocations()) - Visitor.handleNestedNameSpecifierLoc(Location); - return Visitor.getLocationsFound(); }