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