Author: hokein Date: Tue Oct 4 04:05:31 2016 New Revision: 283202 URL: http://llvm.org/viewvc/llvm-project?rev=283202&view=rev Log: [clang-move] Make it support both relative and absolute file path arguments.
Reviewers: ioeric Subscribers: beanz, mgorny, cfe-commits Differential Revision: https://reviews.llvm.org/D24922 Added: clang-tools-extra/trunk/test/clang-move/ clang-tools-extra/trunk/test/clang-move/Inputs/ clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json clang-tools-extra/trunk/test/clang-move/Inputs/test.cpp clang-tools-extra/trunk/test/clang-move/Inputs/test.h clang-tools-extra/trunk/test/clang-move/move-class.cpp Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp clang-tools-extra/trunk/clang-move/ClangMove.h clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp clang-tools-extra/trunk/test/CMakeLists.txt clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=283202&r1=283201&r2=283202&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original) +++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Tue Oct 4 04:05:31 2016 @@ -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 clang { 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 @@ public: 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 @@ void addOrMergeReplacement(const clang:: } } -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 @@ ClangMoveAction::CreateASTConsumer(clang 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"); @@ -198,8 +250,10 @@ ClangMoveTool::ClangMoveTool( 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::run(const ast_matche } } -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 @@ void ClangMoveTool::moveClassDefinitionT 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, OriginalRunningDirectory, + Spec.OldHeader)) NewHeaderDecls.push_back(MovedDecl); else NewCCDecls.push_back(MovedDecl); Modified: clang-tools-extra/trunk/clang-move/ClangMove.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.h?rev=283202&r1=283201&r2=283202&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-move/ClangMove.h (original) +++ clang-tools-extra/trunk/clang-move/ClangMove.h Tue Oct 4 04:05:31 2016 @@ -37,15 +37,20 @@ public: 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); @@ -53,10 +58,20 @@ public: 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 @@ private: 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 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<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 +122,19 @@ class ClangMoveActionFactory : public to 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 Modified: clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp?rev=283202&r1=283201&r2=283202&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp (original) +++ clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp Tue Oct 4 04:05:31 2016 @@ -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 <set> #include <string> @@ -24,6 +25,7 @@ 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::OptionCategory ClangMoveCategory("cl 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 @@ int main(int argc, const char **argv) { 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; Modified: clang-tools-extra/trunk/test/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/CMakeLists.txt?rev=283202&r1=283201&r2=283202&view=diff ============================================================================== --- clang-tools-extra/trunk/test/CMakeLists.txt (original) +++ clang-tools-extra/trunk/test/CMakeLists.txt Tue Oct 4 04:05:31 2016 @@ -44,6 +44,7 @@ set(CLANG_TOOLS_TEST_DEPS clang-apply-replacements clang-change-namespace clang-include-fixer + clang-move clang-query clang-rename clang-reorder-fields Added: clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json?rev=283202&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json (added) +++ clang-tools-extra/trunk/test/clang-move/Inputs/database_template.json Tue Oct 4 04:05:31 2016 @@ -0,0 +1,7 @@ +[ +{ + "directory": "$test_dir/build", + "command": "clang++ -o test.o $test_dir/test.cpp", + "file": "$test_dir/test.cpp" +} +] Added: clang-tools-extra/trunk/test/clang-move/Inputs/test.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-move/Inputs/test.cpp?rev=283202&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-move/Inputs/test.cpp (added) +++ clang-tools-extra/trunk/test/clang-move/Inputs/test.cpp Tue Oct 4 04:05:31 2016 @@ -0,0 +1,8 @@ +#include "test.h" +#include "test2.h" + +namespace a { +int Foo::f() { + return 0; +} +} // namespace a Added: clang-tools-extra/trunk/test/clang-move/Inputs/test.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-move/Inputs/test.h?rev=283202&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-move/Inputs/test.h (added) +++ clang-tools-extra/trunk/test/clang-move/Inputs/test.h Tue Oct 4 04:05:31 2016 @@ -0,0 +1,6 @@ +namespace a { +class Foo { +public: + int f(); +}; +} // namespace a Added: clang-tools-extra/trunk/test/clang-move/move-class.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-move/move-class.cpp?rev=283202&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-move/move-class.cpp (added) +++ clang-tools-extra/trunk/test/clang-move/move-class.cpp Tue Oct 4 04:05:31 2016 @@ -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 Modified: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp?rev=283202&r1=283201&r2=283202&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Tue Oct 4 04:05:31 2016 @@ -156,9 +156,12 @@ runClangMoveOnCode(const move::ClangMove 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", _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits