hokein created this revision. hokein added a reviewer: ioeric. hokein added a subscriber: cfe-commits.
https://reviews.llvm.org/D24922 Files: clang-move/ClangMove.cpp clang-move/ClangMove.h clang-move/tool/ClangMoveMain.cpp test/clang-move/Inputs/database_template.json test/clang-move/Inputs/test.cpp test/clang-move/Inputs/test.h test/clang-move/move-class.cpp unittests/clang-move/ClangMoveTests.cpp
Index: unittests/clang-move/ClangMoveTests.cpp =================================================================== --- unittests/clang-move/ClangMoveTests.cpp +++ unittests/clang-move/ClangMoveTests.cpp @@ -156,9 +156,12 @@ CreateFiles(Spec.OldCC, TestCC); std::map<std::string, tooling::Replacements> FileToReplacements; - ClangMoveTool MoveTool(Spec, FileToReplacements); + llvm::SmallString<128> InitialDirectory; + std::error_code EC = llvm::sys::fs::current_path(InitialDirectory); + assert(!EC); + (void)EC; auto Factory = llvm::make_unique<clang::move::ClangMoveActionFactory>( - Spec, FileToReplacements); + Spec, FileToReplacements, InitialDirectory.str()); tooling::runToolOnCodeWithArgs( Factory->create(), TestCC, {"-std=c++11"}, TestCCName, "clang-move", Index: test/clang-move/move-class.cpp =================================================================== --- /dev/null +++ test/clang-move/move-class.cpp @@ -0,0 +1,39 @@ +// RUN: mkdir -p %T/clang-move/build +// RUN: sed 's|test_dir|%/T/clang-move|g' %S/Inputs/database_template.json > %T/clang-move/compile_commands.json +// RUN: cp %S/Inputs/test* %T/clang-move/ +// RUN: touch test2.h +// RUN: cd %T/clang-move +// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=../clang-move/test.cpp -old_header=../clang-move/test.h %T/clang-move/test.cpp +// RUN: FileCheck -input-file=%T/clang-move/new_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s +// RUN: FileCheck -input-file=%T/clang-move/new_test.h -check-prefix=CHECK-NEW-TEST-H %s +// RUN: FileCheck -input-file=%T/clang-move/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s +// RUN: FileCheck -input-file=%T/clang-move/test.h -check-prefix=CHECK-OLD-TEST-H %s +// +// RUN: cp %S/Inputs/test* %T/clang-move/ +// RUN: cd %T/clang-move +// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=%T/clang-move/test.cpp -old_header=%T/clang-move/test.h %T/clang-move/test.cpp +// RUN: FileCheck -input-file=%T/clang-move/new_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s +// RUN: FileCheck -input-file=%T/clang-move/new_test.h -check-prefix=CHECK-NEW-TEST-H %s +// RUN: FileCheck -input-file=%T/clang-move/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s +// RUN: FileCheck -input-file=%T/clang-move/test.h -check-prefix=CHECK-OLD-TEST-H %s +// +// CHECK-NEW-TEST-H: namespace a { +// CHECK-NEW-TEST-H: class Foo { +// CHECK-NEW-TEST-H: public: +// CHECK-NEW-TEST-H: int f(); +// CHECK-NEW-TEST-H: }; +// CHECK-NEW-TEST-H: } // namespace a +// +// CHECK-NEW-TEST-CPP: #include "{{.*}}new_test.h" +// CHECK-NEW-TEST-CPP: #include "test2.h" +// CHECK-NEW-TEST-CPP: namespace a { +// CHECK-NEW-TEST-CPP: int Foo::f() { return 0; } +// CHECK-NEW-TEST-CPP: } // namespace a +// +// CHECK-OLD-TEST-H: namespace a { +// CHECK-OLD-TEST-H: } // namespace a +// +// CHECK-OLD-TEST-CPP: #include "test.h" +// CHECK-OLD-TEST-CPP: #include "test2.h" +// CHECK-OLD-TEST-CPP: namespace a { +// CHECK-OLD-TEST-CPP: } // namespace a Index: test/clang-move/Inputs/test.h =================================================================== --- /dev/null +++ test/clang-move/Inputs/test.h @@ -0,0 +1,6 @@ +namespace a { +class Foo { +public: + int f(); +}; +} // namespace a Index: test/clang-move/Inputs/test.cpp =================================================================== --- /dev/null +++ test/clang-move/Inputs/test.cpp @@ -0,0 +1,8 @@ +#include "test.h" +#include "test2.h" + +namespace a { +int Foo::f() { + return 0; +} +} // namespace a Index: test/clang-move/Inputs/database_template.json =================================================================== --- /dev/null +++ test/clang-move/Inputs/database_template.json @@ -0,0 +1,7 @@ +[ +{ + "directory": "test_dir/build", + "command": "clang++ -o test.o test_dir/test.cpp", + "file": "test_dir/test.cpp" +} +] Index: clang-move/tool/ClangMoveMain.cpp =================================================================== --- clang-move/tool/ClangMoveMain.cpp +++ clang-move/tool/ClangMoveMain.cpp @@ -17,13 +17,15 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/Process.h" #include "llvm/Support/YAMLTraits.h" +#include "llvm/Support/Path.h" #include <set> #include <string> using namespace clang; using namespace llvm; namespace { + std::error_code CreateNewFile(const llvm::Twine &path) { int fd = 0; if (std::error_code ec = @@ -38,17 +40,23 @@ cl::opt<std::string> Name("name", cl::desc("The name of class being moved."), cl::cat(ClangMoveCategory)); -cl::opt<std::string> OldHeader("old_header", cl::desc("Old header."), - cl::cat(ClangMoveCategory)); +cl::opt<std::string> + OldHeader("old_header", + cl::desc("The relative/absolute file path of old header."), + cl::cat(ClangMoveCategory)); -cl::opt<std::string> OldCC("old_cc", cl::desc("Old CC file."), - cl::cat(ClangMoveCategory)); +cl::opt<std::string> + OldCC("old_cc", cl::desc("The relative/absolute file path of old cc."), + cl::cat(ClangMoveCategory)); -cl::opt<std::string> NewHeader("new_header", cl::desc("New header."), - cl::cat(ClangMoveCategory)); +cl::opt<std::string> + NewHeader("new_header", + cl::desc("The relative/absolute file path of new header."), + cl::cat(ClangMoveCategory)); -cl::opt<std::string> NewCC("new_cc", cl::desc("New CC file."), - cl::cat(ClangMoveCategory)); +cl::opt<std::string> + NewCC("new_cc", cl::desc("The relative/absolute file path of new cc."), + cl::cat(ClangMoveCategory)); cl::opt<std::string> Style("style", @@ -71,8 +79,15 @@ Spec.NewHeader = NewHeader; Spec.OldCC = OldCC; Spec.NewCC = NewCC; + + llvm::SmallString<128> InitialDirectory; + if (std::error_code EC = llvm::sys::fs::current_path(InitialDirectory)) + llvm::report_fatal_error("Cannot detect current path: " + + Twine(EC.message())); + auto Factory = llvm::make_unique<clang::move::ClangMoveActionFactory>( - Spec, Tool.getReplacements()); + Spec, Tool.getReplacements(), InitialDirectory.str()); + int CodeStatus = Tool.run(Factory.get()); if (CodeStatus) return CodeStatus; Index: clang-move/ClangMove.h =================================================================== --- clang-move/ClangMove.h +++ clang-move/ClangMove.h @@ -37,15 +37,20 @@ struct MoveDefinitionSpec { // A fully qualified name, e.g. "X", "a::X". std::string Name; + // The file path of old header, can be relative path and absolute path. std::string OldHeader; + // The file path of old cc, can be relative path and absolute path. std::string OldCC; + // The file path of new header, can be relative path and absolute path. std::string NewHeader; + // The file path of new cc, can be relative path and absolute path. std::string NewCC; }; ClangMoveTool( const MoveDefinitionSpec &MoveSpec, - std::map<std::string, tooling::Replacements> &FileToReplacements); + std::map<std::string, tooling::Replacements> &FileToReplacements, + llvm::StringRef OriginalRunningDirectory); void registerMatchers(ast_matchers::MatchFinder *Finder); @@ -55,8 +60,11 @@ // Add #includes from old.h/cc files. The FileName is where the #include // comes from. - void addIncludes(llvm::StringRef IncludeHeader, bool IsAngled, - llvm::StringRef FileName); + void addIncludes(llvm::StringRef IncludeHeader, + llvm::StringRef SearchPath, + bool IsAngled, + llvm::StringRef FileName, + const SourceManager& SM); private: void removeClassDefinitionInOldFiles(); @@ -74,14 +82,21 @@ std::vector<std::string> HeaderIncludes; // The #includes in old_cc.cc. std::vector<std::string> CCIncludes; + // The original working directory where the local clang-move binary runs. + // + // clang-move will change its current working directory to the build + // directory when analysising the source file. We saves the original working + // directory in order to get the absolute file path for the fields in Spec. + std::string OriginalRunningDirectory; }; class ClangMoveAction : public clang::ASTFrontendAction { public: ClangMoveAction( const ClangMoveTool::MoveDefinitionSpec &spec, - std::map<std::string, tooling::Replacements> &FileToReplacements) - : MoveTool(spec, FileToReplacements) { + std::map<std::string, tooling::Replacements> &FileToReplacements, + llvm::StringRef OriginalRunningDirectory) + : MoveTool(spec, FileToReplacements, OriginalRunningDirectory) { MoveTool.registerMatchers(&MatchFinder); } @@ -100,16 +115,19 @@ public: ClangMoveActionFactory( const ClangMoveTool::MoveDefinitionSpec &Spec, - std::map<std::string, tooling::Replacements> &FileToReplacements) - : Spec(Spec), FileToReplacements(FileToReplacements) {} + std::map<std::string, tooling::Replacements> &FileToReplacements, + llvm::StringRef OriginalRunningDirectory) + : Spec(Spec), FileToReplacements(FileToReplacements), + OriginalRunningDirectory(OriginalRunningDirectory) {} clang::FrontendAction *create() override { - return new ClangMoveAction(Spec, FileToReplacements); + return new ClangMoveAction(Spec, FileToReplacements, OriginalRunningDirectory); } private: const ClangMoveTool::MoveDefinitionSpec &Spec; std::map<std::string, tooling::Replacements> &FileToReplacements; + std::string OriginalRunningDirectory; }; } // namespace move Index: clang-move/ClangMove.cpp =================================================================== --- clang-move/ClangMove.cpp +++ clang-move/ClangMove.cpp @@ -16,13 +16,52 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Rewrite/Core/Rewriter.h" #include "clang/Tooling/Core/Replacement.h" +#include "llvm/Support/Path.h" using namespace clang::ast_matchers; namespace clang { namespace move { namespace { +std::string MakeAbsolutePath(StringRef Dir, StringRef Path) { + if (Path.empty()) + return ""; + llvm::SmallString<128> InitialDirectory(Dir); + llvm::SmallString<128> AbsolutePath(Path); + auto EC = llvm::sys::fs::make_absolute(InitialDirectory, AbsolutePath); + assert(!EC); + (void)EC; + llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true); + return AbsolutePath.str(); +} + +std::string MakeAbsolutePath(const SourceManager& SM, StringRef Path) { + llvm::SmallString<128> AbsolutePath(Path); + auto EC = + SM.getFileManager().getVirtualFileSystem()->makeAbsolute(AbsolutePath); + assert(!EC); + (void)EC; + llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true); + return AbsolutePath.str(); +} + +// Matches AST nodes that are expanded within the givien AbsoluteFilePath. +AST_POLYMORPHIC_MATCHER_P(isExpansionInFile, + AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc), + std::string, AbsoluteFilePath) { + auto &SourceManager = Finder->getASTContext().getSourceManager(); + auto ExpansionLoc = SourceManager.getExpansionLoc(Node.getLocStart()); + if (ExpansionLoc.isInvalid()) + return false; + auto FileEntry = + SourceManager.getFileEntryForID(SourceManager.getFileID(ExpansionLoc)); + if (!FileEntry) + return false; + return MakeAbsolutePath(SourceManager, FileEntry->getName()) == + AbsoluteFilePath; +} + // FIXME: Move to ASTMatcher. AST_POLYMORPHIC_MATCHER(isStatic, AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, VarDecl)) { @@ -39,10 +78,11 @@ StringRef FileName, bool IsAngled, clang::CharSourceRange /*FilenameRange*/, const clang::FileEntry * /*File*/, - StringRef /*SearchPath*/, StringRef /*RelativePath*/, + StringRef SearchPath, StringRef /*RelativePath*/, const clang::Module * /*Imported*/) override { if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc))) - MoveTool->addIncludes(FileName, IsAngled, FileEntry->getName()); + MoveTool->addIncludes(FileName, MakeAbsolutePath(SM, SearchPath), + IsAngled, FileEntry->getName(), SM); } private: @@ -72,15 +112,18 @@ } bool IsInHeaderFile(const clang::SourceManager &SM, const clang::Decl *D, - llvm::StringRef HeaderFile) { - if (HeaderFile.empty()) + llvm::StringRef OldHeader, + llvm::StringRef OriginalRunningDirectory) { + if (OldHeader.empty()) return false; auto ExpansionLoc = SM.getExpansionLoc(D->getLocStart()); if (ExpansionLoc.isInvalid()) return false; - if (const auto *FE = SM.getFileEntryForID(SM.getFileID(ExpansionLoc))) - return llvm::StringRef(FE->getName()).endswith(HeaderFile); + if (const auto *FE = SM.getFileEntryForID(SM.getFileID(ExpansionLoc))) { + return MakeAbsolutePath(SM, FE->getName()) == + MakeAbsolutePath(OriginalRunningDirectory, OldHeader); + } return false; } @@ -192,20 +235,23 @@ return MatchFinder.newASTConsumer(); } - ClangMoveTool::ClangMoveTool( const MoveDefinitionSpec &MoveSpec, - std::map<std::string, tooling::Replacements> &FileToReplacements) - : Spec(MoveSpec), FileToReplacements(FileToReplacements) { + std::map<std::string, tooling::Replacements> &FileToReplacements, + llvm::StringRef OriginalRunningDirectory) + : Spec(MoveSpec), FileToReplacements(FileToReplacements), + OriginalRunningDirectory(OriginalRunningDirectory) { Spec.Name = llvm::StringRef(Spec.Name).ltrim(':'); if (!Spec.NewHeader.empty()) CCIncludes.push_back("#include \"" + Spec.NewHeader + "\"\n"); } void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) { std::string FullyQualifiedName = "::" + Spec.Name; - auto InOldHeader = isExpansionInFileMatching(Spec.OldHeader); - auto InOldCC = isExpansionInFileMatching(Spec.OldCC); + auto InOldHeader = isExpansionInFile( + MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader)); + auto InOldCC = isExpansionInFile( + MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC)); auto InOldFiles = anyOf(InOldHeader, InOldCC); auto InMovedClass = hasDeclContext(cxxRecordDecl(hasName(FullyQualifiedName))); @@ -285,22 +331,30 @@ } } -void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled, - llvm::StringRef FileName) { +void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, + llvm::StringRef SearchPath, + bool IsAngled, + llvm::StringRef FileName, + const SourceManager& SM) { // FIXME: Add old.h to the new.cc/h when the new target has dependencies on // old.h/c. For instance, when moved class uses another class defined in // old.h, the old.h should be added in new.h. - if (!Spec.OldHeader.empty() && - llvm::StringRef(Spec.OldHeader).endswith(IncludeHeader)) + if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader) == + MakeAbsolutePath(SearchPath, IncludeHeader)) return; std::string IncludeLine = IsAngled ? ("#include <" + IncludeHeader + ">\n").str() : ("#include \"" + IncludeHeader + "\"\n").str(); - if (!Spec.OldHeader.empty() && FileName.endswith(Spec.OldHeader)) + + std::string AbsolutePath = MakeAbsolutePath(SM, FileName); + if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader) == + AbsolutePath) { HeaderIncludes.push_back(IncludeLine); - else if (!Spec.OldCC.empty() && FileName.endswith(Spec.OldCC)) + } else if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC) == + AbsolutePath) { CCIncludes.push_back(IncludeLine); + } } void ClangMoveTool::removeClassDefinitionInOldFiles() { @@ -319,7 +373,8 @@ std::vector<MovedDecl> NewHeaderDecls; std::vector<MovedDecl> NewCCDecls; for (const auto &MovedDecl : MovedDecls) { - if (IsInHeaderFile(*MovedDecl.SM, MovedDecl.Decl, Spec.OldHeader)) + if (IsInHeaderFile(*MovedDecl.SM, MovedDecl.Decl, Spec.OldHeader, + OriginalRunningDirectory)) NewHeaderDecls.push_back(MovedDecl); else NewCCDecls.push_back(MovedDecl);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits