[PATCH] D24828: [clang-move] Don't add old_header to the new files.

[Haojian Wu via cfe-commits]

hokein created this revision.
hokein added a reviewer: ioeric.
hokein added a subscriber: cfe-commits.

https://reviews.llvm.org/D24828

Files:
  clang-move/ClangMove.cpp
  clang-move/ClangMove.h
  unittests/clang-move/ClangMoveTests.cpp


Index: unittests/clang-move/ClangMoveTests.cpp
===================================================================
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -119,8 +119,7 @@
                                  "} // namespace b\n"
                                  "} // namespace a\n";
 
-const char ExpectedNewCC[] = "#include \"foo.h\"\n"
-                             "namespace a {\n"
+const char ExpectedNewCC[] = "namespace a {\n"
                              "namespace b {\n"
                              "namespace {\n"
                              "void f1() {}\n"
@@ -181,11 +180,12 @@
   Spec.OldCC = "foo.cc";
   Spec.NewHeader = "new_foo.h";
   Spec.NewCC = "new_foo.cc";
+  std::string ExpectedHeader = "#include \"" + Spec.NewHeader + "\"\n";
   auto Results = runClangMoveOnCode(Spec);
   EXPECT_EQ(ExpectedTestHeader, Results[Spec.OldHeader]);
   EXPECT_EQ(ExpectedTestCC, Results[Spec.OldCC]);
   EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
-  EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
+  EXPECT_EQ(ExpectedHeader + ExpectedNewCC, Results[Spec.NewCC]);
 }
 
 TEST(ClangMove, MoveHeaderOnly) {
@@ -204,10 +204,11 @@
   Spec.Name = "a::b::Foo";
   Spec.OldCC = "foo.cc";
   Spec.NewCC = "new_foo.cc";
+  std::string ExpectedHeader = "#include \"foo.h\"\n";
   auto Results = runClangMoveOnCode(Spec);
   EXPECT_EQ(2u, Results.size());
   EXPECT_EQ(ExpectedTestCC, Results[Spec.OldCC]);
-  EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
+  EXPECT_EQ(ExpectedHeader + ExpectedNewCC, Results[Spec.NewCC]);
 }
 
 TEST(ClangMove, MoveNonExistClass) {
Index: clang-move/ClangMove.h
===================================================================
--- clang-move/ClangMove.h
+++ clang-move/ClangMove.h
@@ -55,7 +55,8 @@
 
   // Add #includes from old.h/cc files. The FileName is where the #include
   // comes from.
-  void addIncludes(llvm::StringRef IncludeLine, llvm::StringRef FileName);
+  void addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
+                   llvm::StringRef FileName);
 
 private:
   void removeClassDefinitionInOldFiles();
Index: clang-move/ClangMove.cpp
===================================================================
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -41,15 +41,8 @@
                           const clang::FileEntry * /*File*/,
                           StringRef /*SearchPath*/, StringRef /*RelativePath*/,
                           const clang::Module * /*Imported*/) override {
-    if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc))) {
-      if (IsAngled) {
-        MoveTool->addIncludes("#include <" + FileName.str() + ">\n",
-                              FileEntry->getName());
-      } else {
-        MoveTool->addIncludes("#include \"" + FileName.str() + "\"\n",
-                              FileEntry->getName());
-      }
-    }
+    if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc)))
+      MoveTool->addIncludes(FileName, IsAngled, FileEntry->getName());
   }
 
 private:
@@ -135,7 +128,7 @@
 
   // Add #Includes.
   std::string AllIncludesString;
-  // FIXME: Filter out the old_header.h and add header guard.
+  // FIXME: Add header guard.
   for (const auto &Include : Includes)
     AllIncludesString += Include;
   clang::tooling::Replacement InsertInclude(FileName, 0, 0, AllIncludesString);
@@ -205,6 +198,8 @@
       std::map<std::string, tooling::Replacements> &FileToReplacements)
       : Spec(MoveSpec), FileToReplacements(FileToReplacements) {
   Spec.Name = llvm::StringRef(Spec.Name).ltrim(':');
+  if (!Spec.NewHeader.empty())
+    CCIncludes.push_back("#include \"" + Spec.NewHeader + "\"\n");
 }
 
 void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
@@ -290,12 +285,19 @@
   }
 }
 
-void ClangMoveTool::addIncludes(llvm::StringRef IncludeLine,
+void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
                                 llvm::StringRef FileName) {
+  // Don't add the old_header.h to the new files.
+  if (!Spec.OldHeader.empty() && IncludeHeader.endswith(Spec.OldHeader))
+    return;
+
+  std::string IncludeLine = IsAngled
+                                ? "#include <" + IncludeHeader.str() + ">\n"
+                                : "#include \"" + IncludeHeader.str() + "\"\n";
   if (!Spec.OldHeader.empty() && FileName.endswith(Spec.OldHeader))
-    HeaderIncludes.push_back(IncludeLine.str());
+    HeaderIncludes.push_back(IncludeLine);
   else if (!Spec.OldCC.empty() && FileName.endswith(Spec.OldCC))
-    CCIncludes.push_back(IncludeLine.str());
+    CCIncludes.push_back(IncludeLine);
 }
 
 void ClangMoveTool::removeClassDefinitionInOldFiles() {

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits