clang-rename: add a -old-name option

[vmiklos]

Summary

This is similar to -offset with the following differences:

1. -offset can refer to local variables as well.

2. -old-name makes it easier to refer to e.g. ClassName::MemberName by

spelling out the fully qualified name, instead of having to use e.g.
grep to look up the exact offset.

In other words, -offset is great when clang-rename is invoked by e.g. an
IDE, but not really user-friendly when the tool is invoked by the user
from commandline. That's the use case where -old-name is supposed to
improve the situation.

[klimek]

clang-rename/USRFinder.h

28–33 ↗

(On Diff #61237)

These seem to be fundamentally different use cases. Can we add a new top-level function instead?
It seems like the new case would have a much simpler implementation.

[vmiklos]

Added a getNamedDeclFor() for the fully qualified name-based search.

[klimek]

LG

clang-tools-extra/trunk/clang-rename/USRFinder.h

 
 namespace rename {
 
 // Given an AST context and a point, returns a NamedDecl identifying the symbol
 // at the point. Returns null if nothing is found at the point.
 const NamedDecl *getNamedDeclAt(const ASTContext &Context,
                                 const SourceLocation Point);
 
+// Given an AST context and a fully qualified name, returns a NamedDecl
+// identifying the symbol with a matching name. Returns null if nothing is
+// found for the name.
+const NamedDecl *getNamedDeclFor(const ASTContext &Context,
+                                 const std::string &Name);
+
 // Converts a Decl into a USR.
 std::string getUSRForDecl(const Decl *Decl);
 
 }
 }
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_FINDER_H

clang-tools-extra/trunk/clang-rename/USRFinder.cpp

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 SourceManager &SourceMgr,
                                       const SourceLocation Point)
       : Result(nullptr), SourceMgr(SourceMgr),
         Point(Point) {
   }
 
+  // \brief Finds the NamedDecl for a name in the source.
+  // \param Name the fully qualified name.
+  explicit NamedDeclFindingASTVisitor(const SourceManager &SourceMgr,
+                                      const std::string &Name)
+      : Result(nullptr), SourceMgr(SourceMgr),
+        Name(Name) {
+  }
+
   // 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 setResult(Decl, Decl->getLocation(),
bool checkNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) {
     }
     return true;
   }
 
   // \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 (Name.empty()) {
+      // Offset is used to find the declaration.
       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()) {
+        return true;
+      }
+    }
     Result = Decl;
     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));
   }
 
   // \brief Determines if the Point is within Start and End.
   bool isPointWithin(const SourceLocation Start, const SourceLocation End) {
     // FIXME: Add tests for Point == End.
     return Point == Start || Point == End ||
            (SourceMgr.isBeforeInTranslationUnit(Start, Point) &&
             SourceMgr.isBeforeInTranslationUnit(Point, End));
   }
 
   const NamedDecl *Result;
   const SourceManager &SourceMgr;
   const SourceLocation Point; // The location to find the NamedDecl.
+  const std::string Name;
 };
 }
 
 const NamedDecl *getNamedDeclAt(const ASTContext &Context,
                                 const SourceLocation Point) {
   const auto &SourceMgr = Context.getSourceManager();
   const auto SearchFile = SourceMgr.getFilename(Point);
 
if (FileName == SearchFile) {
         return Result;
       }
     }
   }
 
   return nullptr;
 }
 
+const NamedDecl *getNamedDeclFor(const ASTContext &Context,
+                                 const std::string &Name) {
+  const auto &SourceMgr = Context.getSourceManager();
+  NamedDeclFindingASTVisitor Visitor(SourceMgr, Name);
+  auto Decls = Context.getTranslationUnitDecl()->decls();
+
+  for (auto &CurrDecl : Decls) {
+    Visitor.TraverseDecl(CurrDecl);
+    if (const NamedDecl *Result = Visitor.getNamedDecl()) {
+      return Result;
+    }
+  }
+
+  return nullptr;
+}
+
 std::string getUSRForDecl(const Decl *Decl) {
   llvm::SmallVector<char, 128> Buff;
 
   // FIXME: Add test for the nullptr case.
   if (Decl == nullptr || index::generateUSRForDecl(Decl, Buff))
     return "";
 
   return std::string(Buff.data(), Buff.size());
 }
 
 } // namespace clang
 } // namespace rename

clang-tools-extra/trunk/clang-rename/USRFindingAction.h

 namespace clang {
 class ASTConsumer;
 class CompilerInstance;
 class NamedDecl;
 
 namespace rename {
 
 struct USRFindingAction {
-  USRFindingAction(unsigned Offset) : SymbolOffset(Offset) {
+  USRFindingAction(unsigned Offset, const std::string &Name) : SymbolOffset(Offset), OldName(Name) {
   }
   std::unique_ptr<ASTConsumer> newASTConsumer();
 
   // \brief get the spelling of the USR(s) as it would appear in source files.
   const std::string &getUSRSpelling() {
     return SpellingName;
   }
 
   const std::vector<std::string> &getUSRs() {
     return USRs;
   }
 
 private:
   unsigned SymbolOffset;
+  std::string OldName;
   std::string SpellingName;
   std::vector<std::string> USRs;
 };
 
 }
 }
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_FINDING_ACTION_H_

clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp

 struct NamedDeclFindingConsumer : public ASTConsumer {
   void HandleTranslationUnit(ASTContext &Context) override {
     const auto &SourceMgr = Context.getSourceManager();
     // The file we look for the USR in will always be the main source file.
     const auto Point = SourceMgr.getLocForStartOfFile(
         SourceMgr.getMainFileID()).getLocWithOffset(SymbolOffset);
     if (!Point.isValid())
       return;
-    const NamedDecl *FoundDecl = getNamedDeclAt(Context, Point);
+    const NamedDecl *FoundDecl = nullptr;
+    if (OldName.empty()) {
+      FoundDecl = getNamedDeclAt(Context, Point);
+    } else {
+      FoundDecl = getNamedDeclFor(Context, OldName);
+    }
     if (FoundDecl == nullptr) {
       FullSourceLoc FullLoc(Point, SourceMgr);
       errs() << "clang-rename: could not find symbol at "
              << SourceMgr.getFilename(Point) << ":"
              << FullLoc.getSpellingLineNumber() << ":"
              << FullLoc.getSpellingColumnNumber() << " (offset " << SymbolOffset
              << ").\n";
       return;
void HandleTranslationUnit(ASTContext &Context) override {
     if (const auto *Record = dyn_cast<CXXRecordDecl>(FoundDecl))
       *USRs = getAllConstructorUSRs(Record);
 
     USRs->push_back(getUSRForDecl(FoundDecl));
     *SpellingName = FoundDecl->getNameAsString();
   }
 
   unsigned SymbolOffset;
+  std::string OldName;
   std::string *SpellingName;
   std::vector<std::string> *USRs;
 };
 
 std::unique_ptr<ASTConsumer>
 USRFindingAction::newASTConsumer() {
   std::unique_ptr<NamedDeclFindingConsumer> Consumer(
       new NamedDeclFindingConsumer);
   SpellingName = "";
   Consumer->SymbolOffset = SymbolOffset;
+  Consumer->OldName = OldName;
   Consumer->USRs = &USRs;
   Consumer->SpellingName = &SpellingName;
   return std::move(Consumer);
 }
 
 } // namespace rename
 } // namespace clang

clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp

NewName(
     "new-name",
     cl::desc("The new name to change the symbol to."),
     cl::cat(ClangRenameCategory));
 static cl::opt<unsigned>
 SymbolOffset(
     "offset",
     cl::desc("Locates the symbol by offset as opposed to <line>:<column>."),
     cl::cat(ClangRenameCategory));
+static cl::opt<std::string>
+OldName(
+    "old-name",
+    cl::desc("The fully qualified name of the symbol, if -offset is not used."),
+    cl::cat(ClangRenameCategory));
 static cl::opt<bool>
 Inplace(
     "i",
     cl::desc("Overwrite edited <file>s."),
     cl::cat(ClangRenameCategory));
 static cl::opt<bool>
 PrintName(
     "pn",
if (NewName.empty()) {
     errs() << "clang-rename: no new name provided.\n\n";
     cl::PrintHelpMessage();
     exit(1);
   }
 
   // Get the USRs.
   auto Files = OP.getSourcePathList();
   tooling::RefactoringTool Tool(OP.getCompilations(), Files);
-  rename::USRFindingAction USRAction(SymbolOffset);
+  rename::USRFindingAction USRAction(SymbolOffset, OldName);
 
   // Find the USRs.
   Tool.run(tooling::newFrontendActionFactory(&USRAction).get());
   const auto &USRs = USRAction.getUSRs();
   const auto &PrevName = USRAction.getUSRSpelling();
 
   if (PrevName.empty())
     // An error should have already been printed.

clang-tools-extra/trunk/test/clang-rename/ClassTestByName.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -old-name=Cla -new-name=Hector %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+class Cla { // CHECK: class Hector
+};
+
+int main() {
+  Cla *Pointer = 0; // CHECK: Hector *Pointer = 0;
+  return 0;
+}