[clang-tools-extra] r287752 - [clang-move] Add some options allowing to add old/new.h to new/old.h respectively.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 23 02:04:20 PST 2016


Author: hokein
Date: Wed Nov 23 04:04:19 2016
New Revision: 287752

URL: http://llvm.org/viewvc/llvm-project?rev=287752&view=rev
Log:
[clang-move] Add some options allowing to add old/new.h to new/old.h respectively.

Summary:
* --new_depend_on_old: new header will include old header
* --old_depend_on_new: old header will include new header.

Reviewers: ioeric

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D26966

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/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=287752&r1=287751&r2=287752&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Wed Nov 23 04:04:19 2016
@@ -150,7 +150,7 @@ public:
     MoveTool->getMovedDecls().emplace_back(D,
                                            &Result.Context->getSourceManager());
     MoveTool->getUnremovedDeclsInOldHeader().erase(D);
-    MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back());
+    MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back());
   }
 
 private:
@@ -181,7 +181,7 @@ private:
     // case.
     if (!CMD->isInlined()) {
       MoveTool->getMovedDecls().emplace_back(CMD, SM);
-      MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back());
+      MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back());
       // Get template class method from its method declaration as
       // UnremovedDecls stores template class method.
       if (const auto *FTD = CMD->getDescribedFunctionTemplate())
@@ -194,7 +194,7 @@ private:
   void MatchClassStaticVariable(const clang::NamedDecl *VD,
                                 clang::SourceManager* SM) {
     MoveTool->getMovedDecls().emplace_back(VD, SM);
-    MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back());
+    MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back());
     MoveTool->getUnremovedDeclsInOldHeader().erase(VD);
   }
 
@@ -206,7 +206,7 @@ private:
       MoveTool->getMovedDecls().emplace_back(TC, SM);
     else
       MoveTool->getMovedDecls().emplace_back(CD, SM);
-    MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back());
+    MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back());
     MoveTool->getUnremovedDeclsInOldHeader().erase(
         MoveTool->getMovedDecls().back().Decl);
   }
@@ -305,7 +305,8 @@ std::vector<std::string> GetNamespaces(c
 clang::tooling::Replacements
 createInsertedReplacements(const std::vector<std::string> &Includes,
                            const std::vector<ClangMoveTool::MovedDecl> &Decls,
-                           llvm::StringRef FileName, bool IsHeader = false) {
+                           llvm::StringRef FileName, bool IsHeader = false,
+                           StringRef OldHeaderInclude = "") {
   std::string NewCode;
   std::string GuardName(FileName);
   if (IsHeader) {
@@ -318,6 +319,7 @@ createInsertedReplacements(const std::ve
     NewCode += "#define " + GuardName + "\n\n";
   }
 
+  NewCode += OldHeaderInclude;
   // Add #Includes.
   for (const auto &Include : Includes)
     NewCode += Include;
@@ -410,6 +412,14 @@ ClangMoveTool::ClangMoveTool(
     CCIncludes.push_back("#include \"" + Spec.NewHeader + "\"\n");
 }
 
+void ClangMoveTool::addRemovedDecl(const MovedDecl &Decl) {
+  const auto &SM = *Decl.SM;
+  auto Loc = Decl.Decl->getLocation();
+  StringRef FilePath = SM.getFilename(Loc);
+  FilePathToFileID[FilePath] = SM.getFileID(Loc);
+  RemovedDecls.push_back(Decl);
+}
+
 void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
   Optional<ast_matchers::internal::Matcher<NamedDecl>> HasAnySymbolNames;
   for (StringRef SymbolName: Spec.Names) {
@@ -575,22 +585,40 @@ void ClangMoveTool::addIncludes(llvm::St
 }
 
 void ClangMoveTool::removeClassDefinitionInOldFiles() {
+  if (RemovedDecls.empty()) return;
   for (const auto &MovedDecl : RemovedDecls) {
     const auto &SM = *MovedDecl.SM;
     auto Range = GetFullRange(&SM, MovedDecl.Decl);
     clang::tooling::Replacement RemoveReplacement(
-        *MovedDecl.SM,
+        SM,
         clang::CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()),
         "");
     std::string FilePath = RemoveReplacement.getFilePath().str();
     auto Err = FileToReplacements[FilePath].add(RemoveReplacement);
-    if (Err) {
+    if (Err)
       llvm::errs() << llvm::toString(std::move(Err)) << "\n";
-      continue;
+  }
+  const SourceManager* SM = RemovedDecls[0].SM;
+
+  // Post process of cleanup around all the replacements.
+  for (auto& FileAndReplacements: FileToReplacements) {
+    StringRef FilePath = FileAndReplacements.first;
+    // Add #include of new header to old header.
+    if (Spec.OldDependOnNew &&
+        MakeAbsolutePath(*SM, FilePath) == makeAbsolutePath(Spec.OldHeader)) {
+      // FIXME: Minimize the include path like include-fixer.
+      std::string IncludeNewH = "#include \""  + Spec.NewHeader + "\"\n";
+      // This replacment for inserting header will be cleaned up at the end.
+      auto Err = FileAndReplacements.second.add(
+          tooling::Replacement(FilePath, UINT_MAX, 0, IncludeNewH));
+      if (Err)
+        llvm::errs() << llvm::toString(std::move(Err)) << "\n";
     }
 
-    llvm::StringRef Code =
-        SM.getBufferData(SM.getFileID(MovedDecl.Decl->getLocation()));
+    auto SI = FilePathToFileID.find(FilePath);
+    // Ignore replacements for new.h/cc.
+    if (SI == FilePathToFileID.end()) continue;
+    llvm::StringRef Code = SM->getBufferData(SI->second);
     format::FormatStyle Style =
         format::getStyle("file", FilePath, FallbackStyle);
     auto CleanReplacements = format::cleanupAroundReplacements(
@@ -615,9 +643,13 @@ void ClangMoveTool::moveClassDefinitionT
       NewCCDecls.push_back(MovedDecl);
   }
 
-  if (!Spec.NewHeader.empty())
+  if (!Spec.NewHeader.empty()) {
+    std::string OldHeaderInclude =
+        Spec.NewDependOnOld ? "#include \"" + Spec.OldHeader + "\"\n" : "";
     FileToReplacements[Spec.NewHeader] = createInsertedReplacements(
-        HeaderIncludes, NewHeaderDecls, Spec.NewHeader, /*IsHeader=*/true);
+        HeaderIncludes, NewHeaderDecls, Spec.NewHeader, /*IsHeader=*/true,
+        OldHeaderInclude);
+  }
   if (!Spec.NewCC.empty())
     FileToReplacements[Spec.NewCC] =
         createInsertedReplacements(CCIncludes, NewCCDecls, Spec.NewCC);

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=287752&r1=287751&r2=287752&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.h (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.h Wed Nov 23 04:04:19 2016
@@ -15,6 +15,7 @@
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/StringMap.h"
 #include <map>
 #include <memory>
 #include <string>
@@ -52,6 +53,12 @@ public:
     std::string NewHeader;
     // The file path of new cc, can be relative path and absolute path.
     std::string NewCC;
+    // Whether old.h depends on new.h. If true, #include "new.h" will be added
+    // in old.h.
+    bool OldDependOnNew = false;
+    // Whether new.h depends on old.h. If true, #include "old.h" will be added
+    // in new.h.
+    bool NewDependOnOld = false;
   };
 
   ClangMoveTool(
@@ -83,7 +90,10 @@ public:
 
   std::vector<MovedDecl> &getMovedDecls() { return MovedDecls; }
 
-  std::vector<MovedDecl> &getRemovedDecls() { return RemovedDecls; }
+  /// Add declarations being removed from old.h/cc. For each declarations, the
+  /// method also records the mapping relationship between the corresponding
+  /// FilePath and its FileID.
+  void addRemovedDecl(const MovedDecl &Decl);
 
   llvm::SmallPtrSet<const NamedDecl *, 8> &getUnremovedDeclsInOldHeader() {
     return UnremovedDeclsInOldHeader;
@@ -127,6 +137,9 @@ private:
   /// #include "old.h") in old.cc,  including the enclosing quotes or angle
   /// brackets.
   clang::CharSourceRange OldHeaderIncludeRange;
+  /// Mapping from FilePath to FileID, which can be used in post processes like
+  /// cleanup around replacements.
+  llvm::StringMap<FileID> FilePathToFileID;
 };
 
 class ClangMoveAction : public clang::ASTFrontendAction {

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=287752&r1=287751&r2=287752&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp (original)
+++ clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp Wed Nov 23 04:04:19 2016
@@ -61,6 +61,20 @@ cl::opt<std::string>
     NewCC("new_cc", cl::desc("The relative/absolute file path of new cc."),
           cl::cat(ClangMoveCategory));
 
+cl::opt<bool> OldDependOnNew(
+    "old_depend_on_new",
+    cl::desc(
+        "Whether old header will depend on new header. If true, clang-move will "
+        "add #include of new header to old header."),
+    cl::init(false), cl::cat(ClangMoveCategory));
+
+cl::opt<bool> NewDependOnOld(
+    "new_depend_on_old",
+    cl::desc(
+        "Whether new header will depend on old header. If true, clang-move will "
+        "add #include of old header to new header."),
+    cl::init(false), cl::cat(ClangMoveCategory));
+
 cl::opt<std::string>
     Style("style",
           cl::desc("The style name used for reformatting. Default is \"llvm\""),
@@ -87,6 +101,13 @@ int main(int argc, const char **argv) {
   tooling::CommonOptionsParser OptionsParser(Argc, RawExtraArgs.get(),
                                              ClangMoveCategory);
 
+  if (OldDependOnNew && NewDependOnOld) {
+    llvm::errs() << "Provide either --old_depend_on_new or "
+                    "--new_depend_on_old. clang-move doesn't support these two "
+                    "options at same time (It will introduce include cycle).\n";
+    return 1;
+  }
+
   tooling::RefactoringTool Tool(OptionsParser.getCompilations(),
                                 OptionsParser.getSourcePathList());
   move::ClangMoveTool::MoveDefinitionSpec Spec;
@@ -95,6 +116,8 @@ int main(int argc, const char **argv) {
   Spec.NewHeader = NewHeader;
   Spec.OldCC = OldCC;
   Spec.NewCC = NewCC;
+  Spec.OldDependOnNew = OldDependOnNew;
+  Spec.NewDependOnOld = NewDependOnOld;
 
   llvm::SmallString<128> InitialDirectory;
   if (std::error_code EC = llvm::sys::fs::current_path(InitialDirectory))

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=287752&r1=287751&r2=287752&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Wed Nov 23 04:04:19 2016
@@ -419,6 +419,54 @@ TEST(ClangMove, WellFormattedCode) {
   EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
 }
 
+TEST(ClangMove, AddDependentNewHeader) {
+  const char TestHeader[] = "class A {};\n"
+                            "class B {};\n";
+  const char TestCode[] = "#include \"foo.h\"\n";
+  const char ExpectedOldHeader[] = "#include \"new_foo.h\"\nclass B {};\n";
+  const char ExpectedNewHeader[] = "#ifndef NEW_FOO_H\n"
+                                   "#define NEW_FOO_H\n"
+                                   "\n"
+                                   "class A {};\n"
+                                   "\n"
+                                   "#endif // NEW_FOO_H\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";
+  Spec.OldDependOnNew = true;
+  auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+  EXPECT_EQ(ExpectedOldHeader, Results[Spec.OldHeader]);
+  EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
+}
+
+TEST(ClangMove, AddDependentOldHeader) {
+  const char TestHeader[] = "class A {};\n"
+                            "class B {};\n";
+  const char TestCode[] = "#include \"foo.h\"\n";
+  const char ExpectedNewHeader[] = "#ifndef NEW_FOO_H\n"
+                                   "#define NEW_FOO_H\n"
+                                   "\n"
+                                   "#include \"foo.h\"\n"
+                                   "\n"
+                                   "class B {};\n"
+                                   "\n"
+                                   "#endif // NEW_FOO_H\n";
+  const char ExpectedOldHeader[] = "class A {};\n";
+  move::ClangMoveTool::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("B");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+  Spec.NewDependOnOld = true;
+  auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+  EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
+  EXPECT_EQ(ExpectedOldHeader, Results[Spec.OldHeader]);
+}
+
 } // namespace
 } // namespce move
 } // namespace clang




More information about the cfe-commits mailing list