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 @@ -14,6 +14,7 @@ #include "clang/Frontend/FrontendAction.h" #include "clang/Tooling/Core/Replacement.h" #include "clang/Tooling/Tooling.h" +#include "llvm/ADT/SmallPtrSet.h" #include #include #include @@ -23,6 +24,9 @@ // FIXME: Make it support more types, e.g. function definitions. // Currently only support moving class definition. +// +// When moving all class declarations in old header, all code in old.h/cc will +// be moved. class ClangMoveTool : public ast_matchers::MatchFinder::MatchCallback { public: // Information about the declaration being moved. @@ -68,14 +72,22 @@ /// \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 IncludeRange The source range for the written file name in #include + /// (i.e. "old.h" for #include "old.h") in old.cc. /// \param SM The SourceManager. void addIncludes(llvm::StringRef IncludeHeader, bool IsAngled, llvm::StringRef SearchPath, llvm::StringRef FileName, + clang::CharSourceRange IncludeFilenameRange, const SourceManager &SM); private: + // Make the Path absolute using the OrignalRunningDirectory if the Path is not + // an absolute path. An empty Path will result in an empty string. + std::string makeAbsolutePath(StringRef Path); + void removeClassDefinitionInOldFiles(); void moveClassDefinitionToNewFiles(); + void moveAll(SourceManager& SM, StringRef OldFile, StringRef NewFile); MoveDefinitionSpec Spec; // The Key is file path, value is the replacements being applied to the file. @@ -97,6 +109,12 @@ std::string OriginalRunningDirectory; // The name of a predefined code style. std::string FallbackStyle; + // The unmoved named declarations in old header. + llvm::SmallPtrSet UnremovedDeclsInOldHeader; + /// The source range for the written file name in #include (i.e. "old.h" for + /// #include "old.h") in old.cc, including the enclosing quotes or angle + /// brackets. + clang::CharSourceRange OldHeaderIncludeRange; }; class ClangMoveAction : public clang::ASTFrontendAction { 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 @@ -122,13 +122,13 @@ void InclusionDirective(clang::SourceLocation HashLoc, const clang::Token & /*IncludeTok*/, StringRef FileName, bool IsAngled, - clang::CharSourceRange /*FilenameRange*/, + clang::CharSourceRange FilenameRange, const clang::FileEntry * /*File*/, StringRef SearchPath, StringRef /*RelativePath*/, const clang::Module * /*Imported*/) override { if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc))) MoveTool->addIncludes(FileName, IsAngled, SearchPath, - FileEntry->getName(), SM); + FileEntry->getName(), FilenameRange, SM); } private: @@ -321,20 +321,42 @@ return; } - auto InOldHeader = isExpansionInFile( - MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader)); - auto InOldCC = - isExpansionInFile(MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC)); + auto InOldHeader = isExpansionInFile(makeAbsolutePath(Spec.OldHeader)); + auto InOldCC = isExpansionInFile(makeAbsolutePath(Spec.OldCC)); auto InOldFiles = anyOf(InOldHeader, InOldCC); auto InMovedClass = hasOutermostEnclosingClass(cxxRecordDecl(*InMovedClassNames)); + auto ForwardDecls = + cxxRecordDecl(unless(anyOf(isImplicit(), isDefinition()))); + + //============================================================================ + // Matchers for old header + //============================================================================ + // Match all top-level named declarations (e.g. function, variable, enum) in + // old header, exclude forward class declarations and namespace declarations. + // + // The old header which contains only one declaration being moved and forward + // declarations is considered to be moved totally. + auto AllDeclsInHeader = namedDecl( + unless(ForwardDecls), unless(namespaceDecl()), + unless(usingDirectiveDecl()), // using namespace decl. + unless(classTemplateDecl(has(ForwardDecls))), // template forward decl. + InOldHeader, + hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl())))); + Finder->addMatcher(AllDeclsInHeader.bind("decls_in_header"), this); + // Match forward declarations in old header. + Finder->addMatcher(namedDecl(ForwardDecls, InOldHeader).bind("fwd_decl"), + this); + + //============================================================================ + // Matchers for old files, including old.h/old.cc + //============================================================================ // Match moved class declarations. auto MovedClass = cxxRecordDecl( InOldFiles, *InMovedClassNames, isDefinition(), hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl()))); Finder->addMatcher(MovedClass.bind("moved_class"), this); - // Match moved class methods (static methods included) which are defined // outside moved class declaration. Finder->addMatcher( @@ -343,6 +365,9 @@ .bind("class_method"), this); + //============================================================================ + // Matchers for old cc + //============================================================================ // Match static member variable definition of the moved class. Finder->addMatcher( varDecl(InMovedClass, InOldCC, isDefinition(), isStaticDataMember()) @@ -374,16 +399,13 @@ varDecl(IsOldCCStaticDefinition))) .bind("static_decls"), this); - - // Match forward declarations in old header. - Finder->addMatcher( - cxxRecordDecl(unless(anyOf(isImplicit(), isDefinition())), InOldHeader) - .bind("fwd_decl"), - this); } void ClangMoveTool::run(const ast_matchers::MatchFinder::MatchResult &Result) { - if (const auto *CMD = + if (const auto *D = + Result.Nodes.getNodeAs("decls_in_header")) { + UnremovedDeclsInOldHeader.insert(D); + } else if (const auto *CMD = Result.Nodes.getNodeAs("class_method")) { // Skip inline class methods. isInline() ast matcher doesn't ignore this // case. @@ -399,6 +421,7 @@ Result.Nodes.getNodeAs("moved_class")) { MovedDecls.emplace_back(class_decl, &Result.Context->getSourceManager()); RemovedDecls.push_back(MovedDecls.back()); + UnremovedDeclsInOldHeader.erase(class_decl); } else if (const auto *FWD = Result.Nodes.getNodeAs("fwd_decl")) { // Skip all forwad declarations which appear after moved class declaration. @@ -421,21 +444,27 @@ } } +std::string ClangMoveTool::makeAbsolutePath(StringRef Path) { + return MakeAbsolutePath(OriginalRunningDirectory, Path); +} + void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled, llvm::StringRef SearchPath, llvm::StringRef FileName, + clang::CharSourceRange IncludeFilenameRange, const SourceManager &SM) { SmallVector HeaderWithSearchPath; llvm::sys::path::append(HeaderWithSearchPath, SearchPath, IncludeHeader); - std::string AbsoluteOldHeader = - MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader); + std::string AbsoluteOldHeader = makeAbsolutePath(Spec.OldHeader); // 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 (AbsoluteOldHeader == MakeAbsolutePath(SM, llvm::StringRef(HeaderWithSearchPath.data(), - HeaderWithSearchPath.size()))) + HeaderWithSearchPath.size()))) { + OldHeaderIncludeRange = IncludeFilenameRange; return; + } std::string IncludeLine = IsAngled ? ("#include <" + IncludeHeader + ">\n").str() @@ -444,8 +473,7 @@ std::string AbsoluteCurrentFile = MakeAbsolutePath(SM, FileName); if (AbsoluteOldHeader == AbsoluteCurrentFile) { HeaderIncludes.push_back(IncludeLine); - } else if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC) == - AbsoluteCurrentFile) { + } else if (makeAbsolutePath(Spec.OldCC) == AbsoluteCurrentFile) { CCIncludes.push_back(IncludeLine); } } @@ -499,9 +527,46 @@ createInsertedReplacements(CCIncludes, NewCCDecls, Spec.NewCC); } +// Move all contents from OldFile to NewFile. +void ClangMoveTool::moveAll(SourceManager &SM, StringRef OldFile, + StringRef NewFile) { + const FileEntry *FE = SM.getFileManager().getFile(makeAbsolutePath(OldFile)); + if (!FE) { + llvm::errs() << "Failed to get file: " << OldFile << "\n"; + return; + } + FileID ID = SM.getOrCreateFileID(FE, SrcMgr::C_User); + auto Begin = SM.getLocForStartOfFile(ID); + auto End = SM.getLocForEndOfFile(ID); + clang::tooling::Replacement RemoveAll ( + SM, clang::CharSourceRange::getCharRange(Begin, End), ""); + std::string FilePath = RemoveAll.getFilePath().str(); + FileToReplacements[FilePath] = clang::tooling::Replacements(RemoveAll); + + StringRef Code = SM.getBufferData(ID); + if (!NewFile.empty()) { + auto AllCode = clang::tooling::Replacements( + clang::tooling::Replacement(NewFile, 0, 0, Code)); + // If we are moving from old.cc, an extra step is required: excluding + // the #include of "old.h", instead, we replace it with #include of "new.h". + if (Spec.NewCC == NewFile && OldHeaderIncludeRange.isValid()) { + AllCode = AllCode.merge( + clang::tooling::Replacements(clang::tooling::Replacement( + SM, OldHeaderIncludeRange, '"' + Spec.NewHeader + '"'))); + } + FileToReplacements[NewFile] = std::move(AllCode); + } +} + void ClangMoveTool::onEndOfTranslationUnit() { if (RemovedDecls.empty()) return; + if (UnremovedDeclsInOldHeader.empty() && !Spec.OldHeader.empty()) { + auto &SM = *RemovedDecls[0].SM; + moveAll(SM, Spec.OldHeader, Spec.NewHeader); + moveAll(SM, Spec.OldCC, Spec.NewCC); + return; + } removeClassDefinitionInOldFiles(); moveClassDefinitionToNewFiles(); } 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 @@ -1,3 +1,5 @@ +#ifndef TEST_H // comment 1 +#define TEST_H namespace a { class Foo { public: @@ -5,3 +7,4 @@ int f2(int a, int b); }; } // namespace a +#endif // TEST_H 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 @@ -9,8 +9,8 @@ // RUN: clang-move -names="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=../src/test.cpp -old_header=../include/test.h %T/clang-move/src/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/src/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s -// RUN: FileCheck -input-file=%T/clang-move/include/test.h %s -implicit-check-not='{{namespace.*}}' +// RUN: FileCheck -input-file=%T/clang-move/src/test.cpp -check-prefix=CHECK-OLD-TEST-EMPTY -allow-empty %s +// RUN: FileCheck -input-file=%T/clang-move/include/test.h -check-prefix=CHECK-OLD-TEST-EMPTY -allow-empty %s // // RUN: cp %S/Inputs/test.h %T/clang-move/include // RUN: cp %S/Inputs/test.cpp %T/clang-move/src @@ -18,12 +18,12 @@ // RUN: clang-move -names="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=%T/clang-move/src/test.cpp -old_header=%T/clang-move/include/test.h %T/clang-move/src/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/src/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s -// RUN: FileCheck -input-file=%T/clang-move/include/test.h %s -implicit-check-not='{{namespace.*}}' +// RUN: FileCheck -input-file=%T/clang-move/src/test.cpp -check-prefix=CHECK-OLD-TEST-EMPTY -allow-empty %s +// RUN: FileCheck -input-file=%T/clang-move/include/test.h -check-prefix=CHECK-OLD-TEST-EMPTY -allow-empty %s // // -// CHECK-NEW-TEST-H: #ifndef {{.*}}CLANG_MOVE_NEW_TEST_H -// CHECK-NEW-TEST-H: #define {{.*}}CLANG_MOVE_NEW_TEST_H +// CHECK-NEW-TEST-H: #ifndef TEST_H // comment 1 +// CHECK-NEW-TEST-H: #define TEST_H // CHECK-NEW-TEST-H: namespace a { // CHECK-NEW-TEST-H: class Foo { // CHECK-NEW-TEST-H: public: @@ -31,7 +31,7 @@ // CHECK-NEW-TEST-H: int f2(int a, int b); // CHECK-NEW-TEST-H: }; // CHECK-NEW-TEST-H: } // namespace a -// CHECK-NEW-TEST-H: #endif // {{.*}}CLANG_MOVE_NEW_TEST_H +// CHECK-NEW-TEST-H: #endif // TEST_H // // CHECK-NEW-TEST-CPP: #include "{{.*}}new_test.h" // CHECK-NEW-TEST-CPP: #include "test2.h" @@ -40,5 +40,4 @@ // CHECK-NEW-TEST-CPP: int Foo::f2(int a, int b) { return a + b; } // CHECK-NEW-TEST-CPP: } // namespace a // -// CHECK-OLD-TEST-CPP: #include "test.h" -// CHECK-OLD-TEST-CPP: #include "test2.h" +// CHECK-OLD-TEST-EMPTY: {{^}}{{$}} 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 @@ -173,24 +173,25 @@ "} // namespace a\n"; std::map -runClangMoveOnCode(const move::ClangMoveTool::MoveDefinitionSpec &Spec) { +runClangMoveOnCode(const move::ClangMoveTool::MoveDefinitionSpec &Spec, + const char *const Header = TestHeader, + const char *const CC = TestCC) { clang::RewriterTestContext Context; std::map FileToFileID; std::vector> FileToSourceText = { - {TestHeaderName, TestHeader}, {TestCCName, TestCC}}; + {TestHeaderName, Header}, {TestCCName, CC}}; auto CreateFiles = [&FileToSourceText, &Context, &FileToFileID]( llvm::StringRef Name, llvm::StringRef Code) { if (!Name.empty()) { - FileToSourceText.emplace_back(Name, Code); FileToFileID[Name] = Context.createInMemoryFile(Name, Code); } }; CreateFiles(Spec.NewCC, ""); CreateFiles(Spec.NewHeader, ""); - CreateFiles(Spec.OldHeader, TestHeader); - CreateFiles(Spec.OldCC, TestCC); + CreateFiles(Spec.OldHeader, Header); + CreateFiles(Spec.OldCC, CC); std::map FileToReplacements; llvm::SmallString<128> InitialDirectory; @@ -201,7 +202,7 @@ Spec, FileToReplacements, InitialDirectory.str(), "LLVM"); tooling::runToolOnCodeWithArgs( - Factory->create(), TestCC, {"-std=c++11", "-fparse-all-comments"}, + Factory->create(), CC, {"-std=c++11", "-fparse-all-comments"}, TestCCName, "clang-move", std::make_shared(), FileToSourceText); formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm"); @@ -263,6 +264,79 @@ EXPECT_EQ(0u, Results.size()); } +TEST(ClangMove, MoveAll) { + std::vector TestHeaders = { + "class A {\npublic:\n int f();\n};", + // forward declaration. + "class B;\nclass A {\npublic:\n int f();\n};", + // template forward declaration. + "template class B;\nclass A {\npublic:\n int f();\n};", + "namespace a {}\nclass A {\npublic:\n int f();\n};", + "namespace a {}\nusing namespace a;\nclass A {\npublic:\n int f();\n};", + }; + const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }"; + 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"; + for (const auto& Header : TestHeaders) { + auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code); + EXPECT_EQ(Header, Results[Spec.NewHeader]); + EXPECT_EQ("", Results[Spec.OldHeader]); + EXPECT_EQ("", Results[Spec.OldCC]); + } +} + +TEST(ClangMove, MoveAllMultipleClasses) { + move::ClangMoveTool::MoveDefinitionSpec Spec; + std::vector TestHeaders = { + "class C;\nclass A {\npublic:\n int f();\n};\nclass B {};", + "class C;\nclass B;\nclass A {\npublic:\n int f();\n};\nclass B {};", + }; + const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }"; + Spec.Names = {std::string("A"), std::string("B")}; + Spec.OldHeader = "foo.h"; + Spec.OldCC = "foo.cc"; + Spec.NewHeader = "new_foo.h"; + Spec.NewCC = "new_foo.cc"; + for (const auto& Header : TestHeaders) { + auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code); + EXPECT_EQ(Header, Results[Spec.NewHeader]); + EXPECT_EQ("", Results[Spec.OldHeader]); + EXPECT_EQ("", Results[Spec.OldCC]); + } +} + +TEST(ClangMove, DontMoveAll) { + const char ExpectedHeader[] = "#ifndef NEW_FOO_H\n" + "#define NEW_FOO_H\n" + "class A {\npublic:\n int f();\n};\n" + "#endif // NEW_FOO_H\n"; + const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }"; + std::vector TestHeaders = { + "typedef int Int;\nclass A {\npublic:\n int f();\n};", + "using Int=int;\nclass A {\npublic:\n int f();\n};", + "class B {};\nclass A {\npublic:\n int f();\n};", + "void f() {};\nclass A {\npublic:\n int f();\n};", + "enum Color { RED };\nclass A {\npublic:\n int f();\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"; + for (const auto& Header : TestHeaders) { + auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code); + EXPECT_EQ(ExpectedHeader, Results[Spec.NewHeader]); + // The expected old header should not contain class A definition. + std::string ExpectedOldHeader = Header.substr(0, Header.size() - 31); + EXPECT_EQ(ExpectedOldHeader, Results[Spec.OldHeader]); + } +} + } // namespace } // namespce move } // namespace clang