Index: clang-move/ClangMove.h =================================================================== --- clang-move/ClangMove.h +++ clang-move/ClangMove.h @@ -15,6 +15,7 @@ #include "clang/Tooling/Core/Replacement.h" #include "clang/Tooling/Tooling.h" #include +#include #include #include @@ -73,11 +74,13 @@ bool IsAngled, llvm::StringRef SearchPath, llvm::StringRef FileName, + clang::CharSourceRange IncludeRange, const SourceManager& SM); private: 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. @@ -99,6 +102,11 @@ std::string OriginalRunningDirectory; // The name of a predefined code style. std::string FallbackStyle; + // The unmoved named declarations in old header. + std::set UnremovedDeclsInOldHeader; + // The source range for the old.h of #include in old.cc, including the + // enclosing quotes or angle brackets. + clang::CharSourceRange OldHeaderIncludeRange; }; class ClangMoveAction : public clang::ASTFrontendAction { Index: clang-move/ClangMove.cpp =================================================================== --- clang-move/ClangMove.cpp +++ clang-move/ClangMove.cpp @@ -121,13 +121,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: @@ -331,12 +331,35 @@ 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, except 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(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( @@ -345,6 +368,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()) @@ -378,16 +404,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. @@ -403,6 +426,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. @@ -429,6 +453,7 @@ bool IsAngled, llvm::StringRef SearchPath, llvm::StringRef FileName, + clang::CharSourceRange IncludeRange, const SourceManager& SM) { SmallVector HeaderWithSearchPath; llvm::sys::path::append(HeaderWithSearchPath, SearchPath, IncludeHeader); @@ -439,8 +464,10 @@ // old.h, the old.h should be added in new.h. if (AbsoluteOldHeader == MakeAbsolutePath(SM, llvm::StringRef(HeaderWithSearchPath.data(), - HeaderWithSearchPath.size()))) + HeaderWithSearchPath.size()))) { + OldHeaderIncludeRange = IncludeRange; return; + } std::string IncludeLine = IsAngled ? ("#include <" + IncludeHeader + ">\n").str() @@ -504,9 +531,47 @@ 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(OriginalRunningDirectory, 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: test/clang-move/Inputs/test.h =================================================================== --- test/clang-move/Inputs/test.h +++ test/clang-move/Inputs/test.h @@ -1,3 +1,5 @@ +#ifndef TEST_H +#define TEST_H namespace a { class Foo { public: @@ -5,3 +7,4 @@ int f2(int a, int b); }; } // namespace a +#endif // TEST_H Index: test/clang-move/move-class.cpp =================================================================== --- test/clang-move/move-class.cpp +++ 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 +// 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: unittests/clang-move/ClangMoveTests.cpp =================================================================== --- unittests/clang-move/ClangMoveTests.cpp +++ 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,49 @@ EXPECT_EQ(0u, Results.size()); } +TEST(ClangMove, MoveAll) { + move::ClangMoveTool::MoveDefinitionSpec Spec; + std::vector TestHeaders = { + "class A {\npublic:\n int f();\n};", + "class B;\nclass A {\npublic:\n int f();\n};", + "template class B;\nclass A {\npublic:\n int f();\n};" + "namespace a {}\nclass A {\npublic:\n int f();\n};", + }; + const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }"; + Spec.Names = {std::string("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]); + } +} + +TEST(ClangMove, DontMoveAll) { + move::ClangMoveTool::MoveDefinitionSpec Spec; + 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};", + "namespace a {}\nusing namespace a;\nclass A {\npublic:\n int f();\n};" + }; + Spec.Names = {std::string("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]); + } +} + } // namespace } // namespce move } // namespace clang