Index: clang-tools-extra/trunk/clang-move/ClangMove.h =================================================================== --- clang-tools-extra/trunk/clang-move/ClangMove.h +++ clang-tools-extra/trunk/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 &FileToReplacements); + std::map &FileToReplacements, + llvm::StringRef OriginalRunningDirectory); void registerMatchers(ast_matchers::MatchFinder *Finder); @@ -53,10 +58,20 @@ void onEndOfTranslationUnit() override; - // 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); + /// Add #includes from old.h/cc files. + /// + /// \param IncludeHeader The name of the file being included, as written in + /// the source code. + /// \param IsAngled Whether the file name was enclosed in angle brackets. + /// \param SearchPath The search path which was used to find the IncludeHeader + /// in the file system. It can be a relative path or an absolute path. + /// \param FileName The name of file where the IncludeHeader comes from. + /// \param SourceManager The SourceManager. + void addIncludes(llvm::StringRef IncludeHeader, + bool IsAngled, + llvm::StringRef SearchPath, + llvm::StringRef FileName, + const SourceManager& SM); private: void removeClassDefinitionInOldFiles(); @@ -74,14 +89,21 @@ std::vector HeaderIncludes; // The #includes in old_cc.cc. std::vector 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 analyzing the source file. We save 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 &FileToReplacements) - : MoveTool(spec, FileToReplacements) { + std::map &FileToReplacements, + llvm::StringRef OriginalRunningDirectory) + : MoveTool(spec, FileToReplacements, OriginalRunningDirectory) { MoveTool.registerMatchers(&MatchFinder); } @@ -100,16 +122,19 @@ public: ClangMoveActionFactory( const ClangMoveTool::MoveDefinitionSpec &Spec, - std::map &FileToReplacements) - : Spec(Spec), FileToReplacements(FileToReplacements) {} + std::map &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 &FileToReplacements; + std::string OriginalRunningDirectory; }; } // namespace move Index: clang-tools-extra/trunk/clang-move/ClangMove.cpp =================================================================== --- clang-tools-extra/trunk/clang-move/ClangMove.cpp +++ clang-tools-extra/trunk/clang-move/ClangMove.cpp @@ -16,6 +16,7 @@ #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; @@ -23,6 +24,52 @@ namespace move { namespace { +// Make the Path absolute using the CurrentDir if the Path is not an absolute +// path. An empty Path will result in an empty string. +std::string MakeAbsolutePath(StringRef CurrentDir, StringRef Path) { + if (Path.empty()) + return ""; + llvm::SmallString<128> InitialDirectory(CurrentDir); + llvm::SmallString<128> AbsolutePath(Path); + if (std::error_code EC = + llvm::sys::fs::make_absolute(InitialDirectory, AbsolutePath)) + llvm::errs() << "Warning: could not make absolute file: '" << EC.message() + << '\n'; + llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true); + return AbsolutePath.str(); +} + +// Make the Path absolute using the current working directory of the given +// SourceManager if the Path is not an absolute path. +// +// The Path can be a path relative to the build directory, or retrieved from +// the SourceManager. +std::string MakeAbsolutePath(const SourceManager& SM, StringRef Path) { + llvm::SmallString<128> AbsolutePath(Path); + if (std::error_code EC = + SM.getFileManager().getVirtualFileSystem()->makeAbsolute(AbsolutePath)) + llvm::errs() << "Warning: could not make absolute file: '" << EC.message() + << '\n'; + llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true); + return AbsolutePath.str(); +} + +// Matches AST nodes that are expanded within the given 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; +} + class FindAllIncludes : public clang::PPCallbacks { public: explicit FindAllIncludes(SourceManager *SM, ClangMoveTool *const MoveTool) @@ -33,10 +80,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, IsAngled, SearchPath, + FileEntry->getName(), SM); } private: @@ -65,16 +113,19 @@ } } -bool IsInHeaderFile(const clang::SourceManager &SM, const clang::Decl *D, - llvm::StringRef HeaderFile) { - if (HeaderFile.empty()) +bool isInHeaderFile(const clang::SourceManager &SM, const clang::Decl *D, + llvm::StringRef OriginalRunningDirectory, + llvm::StringRef OldHeader) { + 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; } @@ -186,11 +237,12 @@ return MatchFinder.newASTConsumer(); } - ClangMoveTool::ClangMoveTool( const MoveDefinitionSpec &MoveSpec, - std::map &FileToReplacements) - : Spec(MoveSpec), FileToReplacements(FileToReplacements) { + std::map &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"); @@ -198,8 +250,10 @@ 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))); @@ -279,22 +333,31 @@ } } -void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled, - llvm::StringRef FileName) { +void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, + bool IsAngled, + llvm::StringRef SearchPath, + llvm::StringRef FileName, + const SourceManager& SM) { + auto AbsoluteSearchPath = MakeAbsolutePath(SM, SearchPath); // 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(AbsoluteSearchPath, 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() { @@ -313,7 +376,8 @@ std::vector NewHeaderDecls; std::vector NewCCDecls; for (const auto &MovedDecl : MovedDecls) { - if (IsInHeaderFile(*MovedDecl.SM, MovedDecl.Decl, Spec.OldHeader)) + if (isInHeaderFile(*MovedDecl.SM, MovedDecl.Decl, OriginalRunningDirectory, + Spec.OldHeader)) NewHeaderDecls.push_back(MovedDecl); else NewCCDecls.push_back(MovedDecl); Index: clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp =================================================================== --- clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp +++ clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp @@ -17,6 +17,7 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/Process.h" #include "llvm/Support/YAMLTraits.h" +#include "llvm/Support/Path.h" #include #include @@ -24,6 +25,7 @@ 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 Name("name", cl::desc("The name of class being moved."), cl::cat(ClangMoveCategory)); -cl::opt OldHeader("old_header", cl::desc("Old header."), - cl::cat(ClangMoveCategory)); +cl::opt + OldHeader("old_header", + cl::desc("The relative/absolute file path of old header."), + cl::cat(ClangMoveCategory)); -cl::opt OldCC("old_cc", cl::desc("Old CC file."), - cl::cat(ClangMoveCategory)); +cl::opt + OldCC("old_cc", cl::desc("The relative/absolute file path of old cc."), + cl::cat(ClangMoveCategory)); -cl::opt NewHeader("new_header", cl::desc("New header."), - cl::cat(ClangMoveCategory)); +cl::opt + NewHeader("new_header", + cl::desc("The relative/absolute file path of new header."), + cl::cat(ClangMoveCategory)); -cl::opt NewCC("new_cc", cl::desc("New CC file."), - cl::cat(ClangMoveCategory)); +cl::opt + NewCC("new_cc", cl::desc("The relative/absolute file path of new cc."), + cl::cat(ClangMoveCategory)); cl::opt 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( - Spec, Tool.getReplacements()); + Spec, Tool.getReplacements(), InitialDirectory.str()); + int CodeStatus = Tool.run(Factory.get()); if (CodeStatus) return CodeStatus; Index: clang-tools-extra/trunk/test/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/test/CMakeLists.txt +++ clang-tools-extra/trunk/test/CMakeLists.txt @@ -44,6 +44,7 @@ clang-apply-replacements clang-change-namespace clang-include-fixer + clang-move clang-query clang-rename clang-reorder-fields Index: clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json =================================================================== --- clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json +++ clang-tools-extra/trunk/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-tools-extra/trunk/test/clang-move/Inputs/test.h =================================================================== --- clang-tools-extra/trunk/test/clang-move/Inputs/test.h +++ clang-tools-extra/trunk/test/clang-move/Inputs/test.h @@ -0,0 +1,6 @@ +namespace a { +class Foo { +public: + int f(); +}; +} // namespace a Index: clang-tools-extra/trunk/test/clang-move/Inputs/test.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-move/Inputs/test.cpp +++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/test/clang-move/move-class.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-move/move-class.cpp +++ clang-tools-extra/trunk/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 %T/clang-move/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: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp +++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp @@ -156,9 +156,12 @@ CreateFiles(Spec.OldCC, TestCC); std::map 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( - Spec, FileToReplacements); + Spec, FileToReplacements, InitialDirectory.str()); tooling::runToolOnCodeWithArgs( Factory->create(), TestCC, {"-std=c++11"}, TestCCName, "clang-move",