omtcyfz updated this revision to Diff 69214.
omtcyfz marked 2 inline comments as done.
omtcyfz added a comment.

Address couple of comments.


https://reviews.llvm.org/D23651

Files:
  clang-rename/USRFindingAction.cpp
  clang-rename/USRFindingAction.h
  clang-rename/tool/ClangRename.cpp

Index: clang-rename/tool/ClangRename.cpp
===================================================================
--- clang-rename/tool/ClangRename.cpp
+++ clang-rename/tool/ClangRename.cpp
@@ -56,24 +56,17 @@
 static int renameAllMain(int argc, const char *argv[]);
 static int helpMain(int argc, const char *argv[]);
 
-/// \brief An oldname -> newname rename.
-struct RenameAllInfo {
-  std::string OldName;
-  unsigned Offset = 0;
-  std::string NewName;
-};
-
-LLVM_YAML_IS_SEQUENCE_VECTOR(RenameAllInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(RenamePairInfo)
 
 namespace llvm {
 namespace yaml {
 
-/// \brief Specialized MappingTraits to describe how a RenameAllInfo is
+/// \brief Specialized MappingTraits to describe how a RenamePairInfo is
 /// (de)serialized.
-template <> struct MappingTraits<RenameAllInfo> {
-  static void mapping(IO &IO, RenameAllInfo &Info) {
-    IO.mapOptional("OldName", Info.OldName);
+template <> struct MappingTraits<RenamePairInfo> {
+  static void mapping(IO &IO, RenamePairInfo &Info) {
     IO.mapOptional("Offset", Info.Offset);
+    IO.mapOptional("OldName", Info.OldName);
     IO.mapRequired("NewName", Info.NewName);
   }
 };
@@ -155,7 +148,7 @@
       return 1;
     }
 
-    std::vector<RenameAllInfo> Infos;
+    std::vector<RenamePairInfo> Infos;
     llvm::yaml::Input YAML(Buffer.get()->getBuffer());
     YAML >> Infos;
     for (const auto &Info : Infos) {
@@ -175,11 +168,11 @@
   }
 
   // Check if NewNames is a valid identifier in C++17.
+  LangOptions Options;
+  Options.CPlusPlus = true;
+  Options.CPlusPlus1z = true;
+  IdentifierTable Table(Options);
   for (const auto &NewName : NewNames) {
-    LangOptions Options;
-    Options.CPlusPlus = true;
-    Options.CPlusPlus1z = true;
-    IdentifierTable Table(Options);
     auto NewNameTokKind = Table.get(NewName).getTokenID();
     if (!tok::isAnyIdentifier(NewNameTokKind)) {
       errs() << "ERROR: new name is not a valid identifier in C++17.\n\n";
@@ -203,39 +196,31 @@
     exit(1);
   }
 
-  std::vector<std::vector<std::string>> USRList;
-  std::vector<std::string> PrevNames;
   auto Files = OP.getSourcePathList();
   tooling::RefactoringTool Tool(OP.getCompilations(), Files);
+
   unsigned Count = OldNames.size() ? OldNames.size() : SymbolOffsets.size();
+  std::vector<RenamePairInfo> Pairs(Count);
   for (unsigned I = 0; I < Count; ++I) {
-    unsigned SymbolOffset = SymbolOffsets.empty() ? 0 : SymbolOffsets[I];
-    const std::string &OldName = OldNames.empty() ? std::string() : OldNames[I];
-
-    // Get the USRs.
-    rename::USRFindingAction USRAction(SymbolOffset, OldName);
-
-    // Find the USRs.
-    Tool.run(tooling::newFrontendActionFactory(&USRAction).get());
-    const auto &USRs = USRAction.getUSRs();
-    USRList.push_back(USRs);
-    const auto &PrevName = USRAction.getUSRSpelling();
-    PrevNames.push_back(PrevName);
-
-    if (PrevName.empty()) {
-      // An error should have already been printed.
-      exit(1);
-    }
+    Pairs[I].Offset = SymbolOffsets.empty() ? 0 : SymbolOffsets[I];
+    Pairs[I].OldName = OldNames.empty() ? "" : OldNames[I];
+  }
 
-    if (PrintName) {
-      errs() << "clang-rename: found name: " << PrevName << '\n';
+  rename::USRFindingAction USRAction(Pairs);
+  Tool.run(tooling::newFrontendActionFactory(&USRAction).get());
+  const std::vector<std::vector<std::string>> &USRList = USRAction.getUSRList();
+  const std::vector<std::string> &PrevNames = USRAction.getUSRSpellings();
+  if (PrintName) {
+    for (const auto &PrevName : PrevNames) {
+      outs() << "clang-rename found name: " << PrevName << '\n';
     }
   }
 
   // Perform the renaming.
   rename::RenamingAction RenameAction(NewNames, PrevNames, USRList,
                                       Tool.getReplacements(), PrintLocations);
-  auto Factory = tooling::newFrontendActionFactory(&RenameAction);
+  std::unique_ptr<tooling::FrontendActionFactory> Factory =
+      tooling::newFrontendActionFactory(&RenameAction);
   int ExitCode;
 
   if (Inplace) {
Index: clang-rename/USRFindingAction.h
===================================================================
--- clang-rename/USRFindingAction.h
+++ clang-rename/USRFindingAction.h
@@ -17,28 +17,31 @@
 
 #include "clang/Frontend/FrontendAction.h"
 
+/// \brief An oldname -> newname rename.
+struct RenamePairInfo {
+  unsigned Offset = 0;
+  std::string OldName;
+  std::string NewName;
+};
+
 namespace clang {
 class ASTConsumer;
 class CompilerInstance;
 class NamedDecl;
 
 namespace rename {
 
 struct USRFindingAction {
-  USRFindingAction(unsigned Offset, const std::string &Name)
-      : SymbolOffset(Offset), OldName(Name) {}
+  USRFindingAction(ArrayRef<RenamePairInfo> Pairs) : Pairs(Pairs) {}
   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; }
+  const std::vector<std::string> &getUSRSpellings() { return SpellingNames; }
+  const std::vector<std::vector<std::string>> &getUSRList() { return USRList; }
 
 private:
-  unsigned SymbolOffset;
-  std::string OldName;
-  std::string SpellingName;
-  std::vector<std::string> USRs;
+  std::vector<RenamePairInfo> Pairs;
+  std::vector<std::string> SpellingNames;
+  std::vector<std::vector<std::string>> USRList;
 };
 
 } // namespace rename
Index: clang-rename/USRFindingAction.cpp
===================================================================
--- clang-rename/USRFindingAction.cpp
+++ clang-rename/USRFindingAction.cpp
@@ -45,11 +45,10 @@
 // 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) {}
+  explicit 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)) {
@@ -67,7 +66,7 @@
     } 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) {
@@ -134,66 +133,69 @@
 
   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 {
+class NamedDeclFindingConsumer : public ASTConsumer {
+public:
+  explicit NamedDeclFindingConsumer(
+      ArrayRef<RenamePairInfo> Pairs, std::vector<std::string> &SpellingNames,
+      std::vector<std::vector<std::string>> &USRList)
+      : Pairs(Pairs), SpellingNames(SpellingNames), USRList(USRList) {}
+
   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.
-    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 (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;
-    }
+    for (unsigned I = 0; I < Pairs.size(); ++I) {
+      unsigned SymbolOffset = Pairs[I].Offset;
+      const std::string &OldName = Pairs[I].OldName;
+      const SourceManager &SourceMgr = Context.getSourceManager();
+      // The file we look for the USR in will always be the main source file.
+      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 (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;
+      }
 
-    // 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();
+      // 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();
+      }
+      SpellingNames.push_back(FoundDecl->getNameAsString());
+      AdditionalUSRFinder Finder(FoundDecl, Context);
+      USRList.push_back(Finder.Find());
     }
-    *SpellingName = FoundDecl->getNameAsString();
-
-    AdditionalUSRFinder Finder(FoundDecl, Context, USRs);
-    Finder.Find();
   }
 
-  unsigned SymbolOffset;
-  std::string OldName;
-  std::string *SpellingName;
-  std::vector<std::string> *USRs;
+private:
+  ArrayRef<RenamePairInfo> Pairs;
+  std::vector<std::string> &SpellingNames;
+  std::vector<std::vector<std::string>> &USRList;
 };
 
 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>(Pairs, SpellingNames,
+                                                     USRList);
 }
 
 } // namespace rename
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to