This revision was automatically updated to reflect the committed changes.
Closed by commit rL286281: [clang-move] Move all code from old.h/cc directly 
when moving all class… (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D26236?vs=77223&id=77233#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26236

Files:
  clang-tools-extra/trunk/clang-move/ClangMove.cpp
  clang-tools-extra/trunk/clang-move/ClangMove.h
  clang-tools-extra/trunk/test/clang-move/Inputs/test.h
  clang-tools-extra/trunk/test/clang-move/move-class.cpp
  clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp

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<std::string, std::string>
-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<llvm::StringRef, clang::FileID> FileToFileID;
   std::vector<std::pair<std::string, std::string>> 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<std::string, tooling::Replacements> 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<PCHContainerOperations>(),
       FileToSourceText);
   formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm");
@@ -263,6 +264,79 @@
   EXPECT_EQ(0u, Results.size());
 }
 
+TEST(ClangMove, MoveAll) {
+  std::vector<std::string> TestHeaders = {
+    "class A {\npublic:\n  int f();\n};",
+    // forward declaration.
+    "class B;\nclass A {\npublic:\n  int f();\n};",
+    // template forward declaration.
+    "template <typename T> 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<std::string> 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<std::string> 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
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,28 +321,53 @@
     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(
       cxxMethodDecl(InOldFiles, ofOutermostEnclosingClass(*InMovedClassNames),
                     isDefinition())
           .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<clang::NamedDecl>("decls_in_header")) {
+    UnremovedDeclsInOldHeader.insert(D);
+  } else if (const auto *CMD =
           Result.Nodes.getNodeAs<clang::CXXMethodDecl>("class_method")) {
     // Skip inline class methods. isInline() ast matcher doesn't ignore this
     // case.
@@ -399,6 +421,7 @@
                  Result.Nodes.getNodeAs<clang::CXXRecordDecl>("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<clang::CXXRecordDecl>("fwd_decl")) {
     // Skip all forwad declarations which appear after moved class declaration.
@@ -421,31 +444,36 @@
   }
 }
 
+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<char, 128> 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()
                : ("#include \"" + IncludeHeader + "\"\n").str();
 
   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/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 <map>
 #include <string>
 #include <vector>
@@ -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<const NamedDecl*, 8> 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/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,7 +1,10 @@
+#ifndef TEST_H // comment 1
+#define TEST_H
 namespace a {
 class Foo {
 public:
   int f();
   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,36 +9,35 @@
 // 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
 // RUN: cd %T/clang-move/build
 // 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:
 // CHECK-NEW-TEST-H:   int f();
 // 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"
 // CHECK-NEW-TEST-CPP: namespace a {
 // CHECK-NEW-TEST-CPP: int Foo::f() { return 0; }
 // 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: {{^}}{{$}}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to