[clang-rename] Merge rename-{at|all} & optimise.

Having both rename-at and rename-all both seems confusing and introduces
unneeded difficulties. After merging rename-at and rename-all maintaining main
function wrappers and custom help becomes redundant while CLI becomes less
confusing.

D24224 (which was the original patch causing buildbot failures) wasn't aware of
bugs caused by passing both -offset and -qualified-name. After D24224 was landed
it caused buildbot failures and therefor I just reverted it.

Two things that make this patch different from D24224 are:

* unittests/clang-rename was deleted, because it is unmaintained and doesn't do
much.
* Passing both `-offset` and `-qualified-name` isn't allowed anymore for the
sake of preventing bugs.

This patch is a trivial enhancement of accepted D24224 revision.

Tested with `ninja check-all`.

Differential Revision: https://reviews.llvm.org/D24567

llvm-svn: 281710

1 files changed, 72 insertions, 43 deletions

diff --git a/clang-tools-extra/clang-rename/USRFindingAction.cpp b/clang-tools-extra/clang-rename/USRFindingAction.cpp
index 1383f8f834c..e04362b969e 100644
--- a/clang-tools-extra/clang-rename/USRFindingAction.cpp
+++ b/clang-tools-extra/clang-rename/USRFindingAction.cpp
@@ -28,6 +28,7 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
+
 #include <algorithm>
 #include <set>
 #include <string>
@@ -45,11 +46,10 @@ namespace {
 // to virtual method.
 class AdditionalUSRFinder : public RecursiveASTVisitor<AdditionalUSRFinder> {
 public:
-  explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
-                               std::vector<std::string> *USRs)
-      : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
+  AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context)
+      : FoundDecl(FoundDecl), Context(Context) {}
 
-  void Find() {
+  std::vector<std::string> Find() {
     // Fill OverriddenMethods and PartialSpecs storages.
     TraverseDecl(Context.getTranslationUnitDecl());
     if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FoundDecl)) {
@@ -66,7 +66,7 @@ public:
     } else {
       USRSet.insert(getUSRForDecl(FoundDecl));
     }
-    USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
+    return std::vector<std::string>(USRSet.begin(), USRSet.end());
   }
 
   bool VisitCXXMethodDecl(const CXXMethodDecl *MethodDecl) {
@@ -129,69 +129,98 @@ private:
 
   const Decl *FoundDecl;
   ASTContext &Context;
-  std::vector<std::string> *USRs;
   std::set<std::string> USRSet;
   std::vector<const CXXMethodDecl *> OverriddenMethods;
   std::vector<const ClassTemplatePartialSpecializationDecl *> PartialSpecs;
 };
 } // namespace
 
-struct NamedDeclFindingConsumer : public ASTConsumer {
-  void HandleTranslationUnit(ASTContext &Context) override {
-    const SourceManager &SourceMgr = Context.getSourceManager();
-    // The file we look for the USR in will always be the main source file.
+class NamedDeclFindingConsumer : public ASTConsumer {
+public:
+  NamedDeclFindingConsumer(ArrayRef<unsigned> SymbolOffsets,
+                           ArrayRef<std::string> QualifiedNames,
+                           std::vector<std::string> &SpellingNames,
+                           std::vector<std::vector<std::string>> &USRList,
+                           bool &ErrorOccurred)
+      : SymbolOffsets(SymbolOffsets), QualifiedNames(QualifiedNames),
+        SpellingNames(SpellingNames), USRList(USRList),
+        ErrorOccurred(ErrorOccurred) {}
+
+private:
+  bool FindSymbol(ASTContext &Context, const SourceManager &SourceMgr,
+                  unsigned SymbolOffset, const std::string &QualifiedName) {
+    DiagnosticsEngine &Engine = Context.getDiagnostics();
+
     const SourceLocation Point =
         SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID())
             .getLocWithOffset(SymbolOffset);
-    if (!Point.isValid())
-      return;
-    const NamedDecl *FoundDecl = nullptr;
-    if (OldName.empty())
-      FoundDecl = getNamedDeclAt(Context, Point);
-    else
-      FoundDecl = getNamedDeclFor(Context, OldName);
+
+    if (!Point.isValid()) {
+      ErrorOccurred = true;
+      unsigned InvalidOffset = Engine.getCustomDiagID(
+          DiagnosticsEngine::Error,
+          "SourceLocation in file %0 at offset %1 is invalid");
+      Engine.Report(Point, InvalidOffset) << SourceMgr.getFilename(Point)
+                                          << SymbolOffset;
+      return false;
+    }
+
+    const NamedDecl *FoundDecl = QualifiedName.empty()
+                                     ? getNamedDeclAt(Context, Point)
+                                     : getNamedDeclFor(Context, QualifiedName);
+
     if (FoundDecl == nullptr) {
-      if (OldName.empty()) {
+      if (QualifiedName.empty()) {
         FullSourceLoc FullLoc(Point, SourceMgr);
-        errs() << "clang-rename: could not find symbol at "
-               << SourceMgr.getFilename(Point) << ":"
-               << FullLoc.getSpellingLineNumber() << ":"
-               << FullLoc.getSpellingColumnNumber() << " (offset "
-               << SymbolOffset << ").\n";
-      } else {
-        errs() << "clang-rename: could not find symbol " << OldName << ".\n";
+        unsigned CouldNotFindSymbolAt = Engine.getCustomDiagID(
+            DiagnosticsEngine::Error,
+            "clang-rename could not find symbol (offset %0)");
+        Engine.Report(Point, CouldNotFindSymbolAt) << SymbolOffset;
+        ErrorOccurred = true;
+        return false;
       }
-      return;
+      unsigned CouldNotFindSymbolNamed = Engine.getCustomDiagID(
+          DiagnosticsEngine::Error, "clang-rename could not find symbol %0");
+      Engine.Report(CouldNotFindSymbolNamed) << QualifiedName;
+      ErrorOccurred = true;
+      return false;
     }
 
-    // If FoundDecl is a constructor or destructor, we want to instead take the
-    // Decl of the corresponding class.
+    // If FoundDecl is a constructor or destructor, we want to instead take
+    // the Decl of the corresponding class.
     if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(FoundDecl))
       FoundDecl = CtorDecl->getParent();
     else if (const auto *DtorDecl = dyn_cast<CXXDestructorDecl>(FoundDecl))
       FoundDecl = DtorDecl->getParent();
 
-    *SpellingName = FoundDecl->getNameAsString();
+    SpellingNames.push_back(FoundDecl->getNameAsString());
+    AdditionalUSRFinder Finder(FoundDecl, Context);
+    USRList.push_back(Finder.Find());
+    return true;
+  }
 
-    AdditionalUSRFinder Finder(FoundDecl, Context, USRs);
-    Finder.Find();
+  void HandleTranslationUnit(ASTContext &Context) override {
+    const SourceManager &SourceMgr = Context.getSourceManager();
+    for (unsigned Offset : SymbolOffsets) {
+      if (!FindSymbol(Context, SourceMgr, Offset, ""))
+        return;
+    }
+    for (const std::string &QualifiedName : QualifiedNames) {
+      if (!FindSymbol(Context, SourceMgr, 0, QualifiedName))
+        return;
+    }
   }
 
-  unsigned SymbolOffset;
-  std::string OldName;
-  std::string *SpellingName;
-  std::vector<std::string> *USRs;
+  ArrayRef<unsigned> SymbolOffsets;
+  ArrayRef<std::string> QualifiedNames;
+  std::vector<std::string> &SpellingNames;
+  std::vector<std::vector<std::string>> &USRList;
+  bool &ErrorOccurred;
 };
 
 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);
+  return llvm::make_unique<NamedDeclFindingConsumer>(
+      SymbolOffsets, QualifiedNames, SpellingNames, USRList, ErrorOccurred);
 }
 
 } // namespace rename