2 files changed, 72 insertions, 197 deletions

diff --git a/clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp b/clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
index f36387dafdb..3bfb5bbe35e 100644
--- a/clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
+++ b/clang/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<NamedDeclFindingASTVisitor> {
+
+/// Recursively visits each AST node to find the symbol underneath the cursor.
+class NamedDeclOccurrenceFindingVisitor
+    : public RecursiveSymbolVisitor<NamedDeclOccurrenceFindingVisitor> {
 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<CXXConversionDecl>(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<TemplateTypeParmType>(Loc.getType()))
-      return setResult(TemplateTypeParm->getDecl(), TypeBeginLoc, TypeEndLoc);
-    if (const auto *TemplateSpecType =
-            dyn_cast<TemplateSpecializationType>(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<SourceRange> 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<ASTContext &>(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<NamedDeclFindingVisitor> {
+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/clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp b/clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
index 934507fe6ea..dc21a94610c 100644
--- a/clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ b/clang/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<USRLocFindingASTVisitor> {
+    : public RecursiveSymbolVisitor<USRLocFindingASTVisitor> {
 public:
   explicit USRLocFindingASTVisitor(const std::vector<std::string> &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<TemplateTypeParmType>(Loc.getType())) {
-      if (USRSet.find(getUSRForDecl(TemplateTypeParm->getDecl())) !=
-          USRSet.end())
-        checkAndAddLocation(Loc.getBeginLoc());
+  bool visitSymbolOccurrence(const NamedDecl *ND,
+                             ArrayRef<SourceRange> 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<std::string> &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();
 }