From 48ac304c8ebc3c924bf427fcdad49ba9a3eb7ab1 Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Wed, 23 Nov 2016 10:04:19 +0000 Subject: [PATCH] [clang-move] Add some options allowing to add old/new.h to new/old.h respectively. Summary: * --new_depend_on_old: new header will include old header * --old_depend_on_new: old header will include new header. Reviewers: ioeric Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D26966 llvm-svn: 287752 --- clang-tools-extra/clang-move/ClangMove.cpp | 56 +++++++++++++++++----- clang-tools-extra/clang-move/ClangMove.h | 15 +++++- .../clang-move/tool/ClangMoveMain.cpp | 23 +++++++++ .../unittests/clang-move/ClangMoveTests.cpp | 48 +++++++++++++++++++ 4 files changed, 129 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clang-move/ClangMove.cpp b/clang-tools-extra/clang-move/ClangMove.cpp index 5e40691..d4d0af8 100644 --- a/clang-tools-extra/clang-move/ClangMove.cpp +++ b/clang-tools-extra/clang-move/ClangMove.cpp @@ -150,7 +150,7 @@ public: MoveTool->getMovedDecls().emplace_back(D, &Result.Context->getSourceManager()); MoveTool->getUnremovedDeclsInOldHeader().erase(D); - MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back()); + MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back()); } private: @@ -181,7 +181,7 @@ private: // case. if (!CMD->isInlined()) { MoveTool->getMovedDecls().emplace_back(CMD, SM); - MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back()); + MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back()); // Get template class method from its method declaration as // UnremovedDecls stores template class method. if (const auto *FTD = CMD->getDescribedFunctionTemplate()) @@ -194,7 +194,7 @@ private: void MatchClassStaticVariable(const clang::NamedDecl *VD, clang::SourceManager* SM) { MoveTool->getMovedDecls().emplace_back(VD, SM); - MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back()); + MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back()); MoveTool->getUnremovedDeclsInOldHeader().erase(VD); } @@ -206,7 +206,7 @@ private: MoveTool->getMovedDecls().emplace_back(TC, SM); else MoveTool->getMovedDecls().emplace_back(CD, SM); - MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back()); + MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back()); MoveTool->getUnremovedDeclsInOldHeader().erase( MoveTool->getMovedDecls().back().Decl); } @@ -305,7 +305,8 @@ std::vector GetNamespaces(const clang::Decl *D) { clang::tooling::Replacements createInsertedReplacements(const std::vector &Includes, const std::vector &Decls, - llvm::StringRef FileName, bool IsHeader = false) { + llvm::StringRef FileName, bool IsHeader = false, + StringRef OldHeaderInclude = "") { std::string NewCode; std::string GuardName(FileName); if (IsHeader) { @@ -318,6 +319,7 @@ createInsertedReplacements(const std::vector &Includes, NewCode += "#define " + GuardName + "\n\n"; } + NewCode += OldHeaderInclude; // Add #Includes. for (const auto &Include : Includes) NewCode += Include; @@ -410,6 +412,14 @@ ClangMoveTool::ClangMoveTool( CCIncludes.push_back("#include \"" + Spec.NewHeader + "\"\n"); } +void ClangMoveTool::addRemovedDecl(const MovedDecl &Decl) { + const auto &SM = *Decl.SM; + auto Loc = Decl.Decl->getLocation(); + StringRef FilePath = SM.getFilename(Loc); + FilePathToFileID[FilePath] = SM.getFileID(Loc); + RemovedDecls.push_back(Decl); +} + void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) { Optional> HasAnySymbolNames; for (StringRef SymbolName: Spec.Names) { @@ -575,22 +585,40 @@ void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled, } void ClangMoveTool::removeClassDefinitionInOldFiles() { + if (RemovedDecls.empty()) return; for (const auto &MovedDecl : RemovedDecls) { const auto &SM = *MovedDecl.SM; auto Range = GetFullRange(&SM, MovedDecl.Decl); clang::tooling::Replacement RemoveReplacement( - *MovedDecl.SM, + SM, clang::CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()), ""); std::string FilePath = RemoveReplacement.getFilePath().str(); auto Err = FileToReplacements[FilePath].add(RemoveReplacement); - if (Err) { + if (Err) llvm::errs() << llvm::toString(std::move(Err)) << "\n"; - continue; + } + const SourceManager* SM = RemovedDecls[0].SM; + + // Post process of cleanup around all the replacements. + for (auto& FileAndReplacements: FileToReplacements) { + StringRef FilePath = FileAndReplacements.first; + // Add #include of new header to old header. + if (Spec.OldDependOnNew && + MakeAbsolutePath(*SM, FilePath) == makeAbsolutePath(Spec.OldHeader)) { + // FIXME: Minimize the include path like include-fixer. + std::string IncludeNewH = "#include \"" + Spec.NewHeader + "\"\n"; + // This replacment for inserting header will be cleaned up at the end. + auto Err = FileAndReplacements.second.add( + tooling::Replacement(FilePath, UINT_MAX, 0, IncludeNewH)); + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; } - llvm::StringRef Code = - SM.getBufferData(SM.getFileID(MovedDecl.Decl->getLocation())); + auto SI = FilePathToFileID.find(FilePath); + // Ignore replacements for new.h/cc. + if (SI == FilePathToFileID.end()) continue; + llvm::StringRef Code = SM->getBufferData(SI->second); format::FormatStyle Style = format::getStyle("file", FilePath, FallbackStyle); auto CleanReplacements = format::cleanupAroundReplacements( @@ -615,9 +643,13 @@ void ClangMoveTool::moveClassDefinitionToNewFiles() { NewCCDecls.push_back(MovedDecl); } - if (!Spec.NewHeader.empty()) + if (!Spec.NewHeader.empty()) { + std::string OldHeaderInclude = + Spec.NewDependOnOld ? "#include \"" + Spec.OldHeader + "\"\n" : ""; FileToReplacements[Spec.NewHeader] = createInsertedReplacements( - HeaderIncludes, NewHeaderDecls, Spec.NewHeader, /*IsHeader=*/true); + HeaderIncludes, NewHeaderDecls, Spec.NewHeader, /*IsHeader=*/true, + OldHeaderInclude); + } if (!Spec.NewCC.empty()) FileToReplacements[Spec.NewCC] = createInsertedReplacements(CCIncludes, NewCCDecls, Spec.NewCC); diff --git a/clang-tools-extra/clang-move/ClangMove.h b/clang-tools-extra/clang-move/ClangMove.h index 0fa4535..b7aed73 100644 --- a/clang-tools-extra/clang-move/ClangMove.h +++ b/clang-tools-extra/clang-move/ClangMove.h @@ -15,6 +15,7 @@ #include "clang/Tooling/Core/Replacement.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/SmallPtrSet.h" +#include "llvm/ADT/StringMap.h" #include #include #include @@ -52,6 +53,12 @@ public: std::string NewHeader; // The file path of new cc, can be relative path and absolute path. std::string NewCC; + // Whether old.h depends on new.h. If true, #include "new.h" will be added + // in old.h. + bool OldDependOnNew = false; + // Whether new.h depends on old.h. If true, #include "old.h" will be added + // in new.h. + bool NewDependOnOld = false; }; ClangMoveTool( @@ -83,7 +90,10 @@ public: std::vector &getMovedDecls() { return MovedDecls; } - std::vector &getRemovedDecls() { return RemovedDecls; } + /// Add declarations being removed from old.h/cc. For each declarations, the + /// method also records the mapping relationship between the corresponding + /// FilePath and its FileID. + void addRemovedDecl(const MovedDecl &Decl); llvm::SmallPtrSet &getUnremovedDeclsInOldHeader() { return UnremovedDeclsInOldHeader; @@ -127,6 +137,9 @@ private: /// #include "old.h") in old.cc, including the enclosing quotes or angle /// brackets. clang::CharSourceRange OldHeaderIncludeRange; + /// Mapping from FilePath to FileID, which can be used in post processes like + /// cleanup around replacements. + llvm::StringMap FilePathToFileID; }; class ClangMoveAction : public clang::ASTFrontendAction { diff --git a/clang-tools-extra/clang-move/tool/ClangMoveMain.cpp b/clang-tools-extra/clang-move/tool/ClangMoveMain.cpp index 016c56f..62bf98d 100644 --- a/clang-tools-extra/clang-move/tool/ClangMoveMain.cpp +++ b/clang-tools-extra/clang-move/tool/ClangMoveMain.cpp @@ -61,6 +61,20 @@ cl::opt NewCC("new_cc", cl::desc("The relative/absolute file path of new cc."), cl::cat(ClangMoveCategory)); +cl::opt OldDependOnNew( + "old_depend_on_new", + cl::desc( + "Whether old header will depend on new header. If true, clang-move will " + "add #include of new header to old header."), + cl::init(false), cl::cat(ClangMoveCategory)); + +cl::opt NewDependOnOld( + "new_depend_on_old", + cl::desc( + "Whether new header will depend on old header. If true, clang-move will " + "add #include of old header to new header."), + cl::init(false), cl::cat(ClangMoveCategory)); + cl::opt Style("style", cl::desc("The style name used for reformatting. Default is \"llvm\""), @@ -87,6 +101,13 @@ int main(int argc, const char **argv) { tooling::CommonOptionsParser OptionsParser(Argc, RawExtraArgs.get(), ClangMoveCategory); + if (OldDependOnNew && NewDependOnOld) { + llvm::errs() << "Provide either --old_depend_on_new or " + "--new_depend_on_old. clang-move doesn't support these two " + "options at same time (It will introduce include cycle).\n"; + return 1; + } + tooling::RefactoringTool Tool(OptionsParser.getCompilations(), OptionsParser.getSourcePathList()); move::ClangMoveTool::MoveDefinitionSpec Spec; @@ -95,6 +116,8 @@ int main(int argc, const char **argv) { Spec.NewHeader = NewHeader; Spec.OldCC = OldCC; Spec.NewCC = NewCC; + Spec.OldDependOnNew = OldDependOnNew; + Spec.NewDependOnOld = NewDependOnOld; llvm::SmallString<128> InitialDirectory; if (std::error_code EC = llvm::sys::fs::current_path(InitialDirectory)) diff --git a/clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp b/clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp index d0dd74f..1b79314 100644 --- a/clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp +++ b/clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp @@ -419,6 +419,54 @@ TEST(ClangMove, WellFormattedCode) { EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]); } +TEST(ClangMove, AddDependentNewHeader) { + const char TestHeader[] = "class A {};\n" + "class B {};\n"; + const char TestCode[] = "#include \"foo.h\"\n"; + const char ExpectedOldHeader[] = "#include \"new_foo.h\"\nclass B {};\n"; + const char ExpectedNewHeader[] = "#ifndef NEW_FOO_H\n" + "#define NEW_FOO_H\n" + "\n" + "class A {};\n" + "\n" + "#endif // NEW_FOO_H\n"; + move::ClangMoveTool::MoveDefinitionSpec Spec; + Spec.Names.push_back("A"); + Spec.OldHeader = "foo.h"; + Spec.OldCC = "foo.cc"; + Spec.NewHeader = "new_foo.h"; + Spec.NewCC = "new_foo.cc"; + Spec.OldDependOnNew = true; + auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode); + EXPECT_EQ(ExpectedOldHeader, Results[Spec.OldHeader]); + EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]); +} + +TEST(ClangMove, AddDependentOldHeader) { + const char TestHeader[] = "class A {};\n" + "class B {};\n"; + const char TestCode[] = "#include \"foo.h\"\n"; + const char ExpectedNewHeader[] = "#ifndef NEW_FOO_H\n" + "#define NEW_FOO_H\n" + "\n" + "#include \"foo.h\"\n" + "\n" + "class B {};\n" + "\n" + "#endif // NEW_FOO_H\n"; + const char ExpectedOldHeader[] = "class A {};\n"; + move::ClangMoveTool::MoveDefinitionSpec Spec; + Spec.Names.push_back("B"); + Spec.OldHeader = "foo.h"; + Spec.OldCC = "foo.cc"; + Spec.NewHeader = "new_foo.h"; + Spec.NewCC = "new_foo.cc"; + Spec.NewDependOnOld = true; + auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode); + EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]); + EXPECT_EQ(ExpectedOldHeader, Results[Spec.OldHeader]); +} + } // namespace } // namespce move } // namespace clang -- 2.7.4