hokein updated this revision to Diff 72613.
hokein added a comment.

Rebase to master


https://reviews.llvm.org/D24922

Files:
  clang-move/ClangMove.cpp
  clang-move/ClangMove.h
  clang-move/tool/ClangMoveMain.cpp
  test/clang-move/Inputs/database_template.json
  test/clang-move/Inputs/test.cpp
  test/clang-move/Inputs/test.h
  test/clang-move/move-class.cpp
  unittests/clang-move/ClangMoveTests.cpp

Index: unittests/clang-move/ClangMoveTests.cpp
===================================================================
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -156,9 +156,12 @@
   CreateFiles(Spec.OldCC, TestCC);
 
   std::map<std::string, tooling::Replacements> FileToReplacements;
-  ClangMoveTool MoveTool(Spec, FileToReplacements);
+  llvm::SmallString<128> InitialDirectory;
+  std::error_code EC = llvm::sys::fs::current_path(InitialDirectory);
+  assert(!EC);
+  (void)EC;
   auto Factory = llvm::make_unique<clang::move::ClangMoveActionFactory>(
-      Spec, FileToReplacements);
+      Spec, FileToReplacements, InitialDirectory.str());
 
   tooling::runToolOnCodeWithArgs(
       Factory->create(), TestCC, {"-std=c++11"}, TestCCName, "clang-move",
Index: test/clang-move/move-class.cpp
===================================================================
--- /dev/null
+++ test/clang-move/move-class.cpp
@@ -0,0 +1,39 @@
+// RUN: mkdir -p %T/clang-move/build
+// RUN: sed 's|test_dir|%/T/clang-move|g' %S/Inputs/database_template.json > %T/clang-move/compile_commands.json
+// RUN: cp %S/Inputs/test*  %T/clang-move/
+// RUN: touch test2.h
+// RUN: cd %T/clang-move
+// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=../clang-move/test.cpp -old_header=../clang-move/test.h %T/clang-move/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/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
+// RUN: FileCheck -input-file=%T/clang-move/test.h -check-prefix=CHECK-OLD-TEST-H %s
+//
+// RUN: cp %S/Inputs/test*  %T/clang-move/
+// RUN: cd %T/clang-move
+// RUN: clang-move -name="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=%T/clang-move/test.cpp -old_header=%T/clang-move/test.h %T/clang-move/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/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
+// RUN: FileCheck -input-file=%T/clang-move/test.h -check-prefix=CHECK-OLD-TEST-H %s
+//
+// 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: };
+// CHECK-NEW-TEST-H: } // namespace a
+//
+// 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: } // namespace a
+//
+// CHECK-OLD-TEST-H: namespace a {
+// CHECK-OLD-TEST-H: } // namespace a
+//
+// CHECK-OLD-TEST-CPP: #include "test.h"
+// CHECK-OLD-TEST-CPP: #include "test2.h"
+// CHECK-OLD-TEST-CPP: namespace a {
+// CHECK-OLD-TEST-CPP: } // namespace a
Index: test/clang-move/Inputs/test.h
===================================================================
--- /dev/null
+++ test/clang-move/Inputs/test.h
@@ -0,0 +1,6 @@
+namespace a {
+class Foo {
+public:
+  int f();
+};
+} // namespace a
Index: test/clang-move/Inputs/test.cpp
===================================================================
--- /dev/null
+++ test/clang-move/Inputs/test.cpp
@@ -0,0 +1,8 @@
+#include "test.h"
+#include "test2.h"
+
+namespace a {
+int Foo::f() {
+  return 0;
+}
+} // namespace a
Index: test/clang-move/Inputs/database_template.json
===================================================================
--- /dev/null
+++ test/clang-move/Inputs/database_template.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "test_dir/build",
+  "command": "clang++ -o test.o test_dir/test.cpp",
+  "file": "test_dir/test.cpp"
+}
+]
Index: clang-move/tool/ClangMoveMain.cpp
===================================================================
--- clang-move/tool/ClangMoveMain.cpp
+++ clang-move/tool/ClangMoveMain.cpp
@@ -17,13 +17,15 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "llvm/Support/Path.h"
 #include <set>
 #include <string>
 
 using namespace clang;
 using namespace llvm;
 
 namespace {
+
 std::error_code CreateNewFile(const llvm::Twine &path) {
   int fd = 0;
   if (std::error_code ec =
@@ -38,17 +40,23 @@
 cl::opt<std::string> Name("name", cl::desc("The name of class being moved."),
                           cl::cat(ClangMoveCategory));
 
-cl::opt<std::string> OldHeader("old_header", cl::desc("Old header."),
-                               cl::cat(ClangMoveCategory));
+cl::opt<std::string>
+    OldHeader("old_header",
+              cl::desc("The relative/absolute file path of old header."),
+              cl::cat(ClangMoveCategory));
 
-cl::opt<std::string> OldCC("old_cc", cl::desc("Old CC file."),
-                           cl::cat(ClangMoveCategory));
+cl::opt<std::string>
+    OldCC("old_cc", cl::desc("The relative/absolute file path of old cc."),
+          cl::cat(ClangMoveCategory));
 
-cl::opt<std::string> NewHeader("new_header", cl::desc("New header."),
-                               cl::cat(ClangMoveCategory));
+cl::opt<std::string>
+    NewHeader("new_header",
+              cl::desc("The relative/absolute file path of new header."),
+              cl::cat(ClangMoveCategory));
 
-cl::opt<std::string> NewCC("new_cc", cl::desc("New CC file."),
-                           cl::cat(ClangMoveCategory));
+cl::opt<std::string>
+    NewCC("new_cc", cl::desc("The relative/absolute file path of new cc."),
+          cl::cat(ClangMoveCategory));
 
 cl::opt<std::string>
     Style("style",
@@ -71,8 +79,15 @@
   Spec.NewHeader = NewHeader;
   Spec.OldCC = OldCC;
   Spec.NewCC = NewCC;
+
+  llvm::SmallString<128> InitialDirectory;
+  if (std::error_code EC = llvm::sys::fs::current_path(InitialDirectory))
+    llvm::report_fatal_error("Cannot detect current path: " +
+                             Twine(EC.message()));
+
   auto Factory = llvm::make_unique<clang::move::ClangMoveActionFactory>(
-      Spec, Tool.getReplacements());
+      Spec, Tool.getReplacements(), InitialDirectory.str());
+
   int CodeStatus = Tool.run(Factory.get());
   if (CodeStatus)
     return CodeStatus;
Index: clang-move/ClangMove.h
===================================================================
--- clang-move/ClangMove.h
+++ clang-move/ClangMove.h
@@ -37,15 +37,20 @@
   struct MoveDefinitionSpec {
     // A fully qualified name, e.g. "X", "a::X".
     std::string Name;
+    // The file path of old header, can be relative path and absolute path.
     std::string OldHeader;
+    // The file path of old cc, can be relative path and absolute path.
     std::string OldCC;
+    // The file path of new header, can be relative path and absolute path.
     std::string NewHeader;
+    // The file path of new cc, can be relative path and absolute path.
     std::string NewCC;
   };
 
   ClangMoveTool(
       const MoveDefinitionSpec &MoveSpec,
-      std::map<std::string, tooling::Replacements> &FileToReplacements);
+      std::map<std::string, tooling::Replacements> &FileToReplacements,
+      llvm::StringRef OriginalRunningDirectory);
 
   void registerMatchers(ast_matchers::MatchFinder *Finder);
 
@@ -55,8 +60,11 @@
 
   // Add #includes from old.h/cc files. The FileName is where the #include
   // comes from.
-  void addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
-                   llvm::StringRef FileName);
+  void addIncludes(llvm::StringRef IncludeHeader,
+                   llvm::StringRef SearchPath,
+                   bool IsAngled,
+                   llvm::StringRef FileName,
+                   const SourceManager& SM);
 
 private:
   void removeClassDefinitionInOldFiles();
@@ -74,14 +82,21 @@
   std::vector<std::string> HeaderIncludes;
   // The #includes in old_cc.cc.
   std::vector<std::string> CCIncludes;
+  // The original working directory where the local clang-move binary runs.
+  //
+  // clang-move will change its current working directory to the build
+  // directory when analysising the source file. We saves the original working
+  // directory in order to get the absolute file path for the fields in Spec.
+  std::string OriginalRunningDirectory;
 };
 
 class ClangMoveAction : public clang::ASTFrontendAction {
 public:
   ClangMoveAction(
       const ClangMoveTool::MoveDefinitionSpec &spec,
-      std::map<std::string, tooling::Replacements> &FileToReplacements)
-      : MoveTool(spec, FileToReplacements) {
+      std::map<std::string, tooling::Replacements> &FileToReplacements,
+      llvm::StringRef OriginalRunningDirectory)
+      : MoveTool(spec, FileToReplacements, OriginalRunningDirectory) {
     MoveTool.registerMatchers(&MatchFinder);
   }
 
@@ -100,16 +115,19 @@
 public:
   ClangMoveActionFactory(
       const ClangMoveTool::MoveDefinitionSpec &Spec,
-      std::map<std::string, tooling::Replacements> &FileToReplacements)
-      : Spec(Spec), FileToReplacements(FileToReplacements) {}
+      std::map<std::string, tooling::Replacements> &FileToReplacements,
+      llvm::StringRef OriginalRunningDirectory)
+      : Spec(Spec), FileToReplacements(FileToReplacements),
+        OriginalRunningDirectory(OriginalRunningDirectory) {}
 
   clang::FrontendAction *create() override {
-    return new ClangMoveAction(Spec, FileToReplacements);
+    return new ClangMoveAction(Spec, FileToReplacements, OriginalRunningDirectory);
   }
 
 private:
   const ClangMoveTool::MoveDefinitionSpec &Spec;
   std::map<std::string, tooling::Replacements> &FileToReplacements;
+  std::string OriginalRunningDirectory;
 };
 
 } // namespace move
Index: clang-move/ClangMove.cpp
===================================================================
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -16,13 +16,52 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace move {
 namespace {
 
+std::string MakeAbsolutePath(StringRef Dir, StringRef Path) {
+  if (Path.empty())
+    return "";
+  llvm::SmallString<128> InitialDirectory(Dir);
+  llvm::SmallString<128> AbsolutePath(Path);
+  auto EC = llvm::sys::fs::make_absolute(InitialDirectory, AbsolutePath);
+  assert(!EC);
+  (void)EC;
+  llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true);
+  return AbsolutePath.str();
+}
+
+std::string MakeAbsolutePath(const SourceManager& SM, StringRef Path) {
+  llvm::SmallString<128> AbsolutePath(Path);
+  auto EC =
+      SM.getFileManager().getVirtualFileSystem()->makeAbsolute(AbsolutePath);
+  assert(!EC);
+  (void)EC;
+  llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true);
+  return AbsolutePath.str();
+}
+
+// Matches AST nodes that are expanded within the givien AbsoluteFilePath.
+AST_POLYMORPHIC_MATCHER_P(isExpansionInFile,
+                          AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc),
+                          std::string, AbsoluteFilePath) {
+  auto &SourceManager = Finder->getASTContext().getSourceManager();
+  auto ExpansionLoc = SourceManager.getExpansionLoc(Node.getLocStart());
+  if (ExpansionLoc.isInvalid())
+    return false;
+  auto FileEntry =
+      SourceManager.getFileEntryForID(SourceManager.getFileID(ExpansionLoc));
+  if (!FileEntry)
+    return false;
+  return MakeAbsolutePath(SourceManager, FileEntry->getName()) ==
+         AbsoluteFilePath;
+}
+
 class FindAllIncludes : public clang::PPCallbacks {
 public:
   explicit FindAllIncludes(SourceManager *SM, ClangMoveTool *const MoveTool)
@@ -33,10 +72,11 @@
                           StringRef FileName, bool IsAngled,
                           clang::CharSourceRange /*FilenameRange*/,
                           const clang::FileEntry * /*File*/,
-                          StringRef /*SearchPath*/, StringRef /*RelativePath*/,
+                          StringRef SearchPath, StringRef /*RelativePath*/,
                           const clang::Module * /*Imported*/) override {
     if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc)))
-      MoveTool->addIncludes(FileName, IsAngled, FileEntry->getName());
+      MoveTool->addIncludes(FileName, MakeAbsolutePath(SM, SearchPath),
+                            IsAngled, FileEntry->getName(), SM);
   }
 
 private:
@@ -66,15 +106,18 @@
 }
 
 bool IsInHeaderFile(const clang::SourceManager &SM, const clang::Decl *D,
-                    llvm::StringRef HeaderFile) {
-  if (HeaderFile.empty())
+                    llvm::StringRef OldHeader,
+                    llvm::StringRef OriginalRunningDirectory) {
+  if (OldHeader.empty())
     return false;
   auto ExpansionLoc = SM.getExpansionLoc(D->getLocStart());
   if (ExpansionLoc.isInvalid())
     return false;
 
-  if (const auto *FE = SM.getFileEntryForID(SM.getFileID(ExpansionLoc)))
-    return llvm::StringRef(FE->getName()).endswith(HeaderFile);
+  if (const auto *FE = SM.getFileEntryForID(SM.getFileID(ExpansionLoc))) {
+    return MakeAbsolutePath(SM, FE->getName()) ==
+           MakeAbsolutePath(OriginalRunningDirectory, OldHeader);
+  }
 
   return false;
 }
@@ -186,20 +229,23 @@
   return MatchFinder.newASTConsumer();
 }
 
-
 ClangMoveTool::ClangMoveTool(
       const MoveDefinitionSpec &MoveSpec,
-      std::map<std::string, tooling::Replacements> &FileToReplacements)
-      : Spec(MoveSpec), FileToReplacements(FileToReplacements) {
+      std::map<std::string, tooling::Replacements> &FileToReplacements,
+      llvm::StringRef OriginalRunningDirectory)
+      : Spec(MoveSpec), FileToReplacements(FileToReplacements),
+        OriginalRunningDirectory(OriginalRunningDirectory) {
   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) {
   std::string FullyQualifiedName = "::" + Spec.Name;
-  auto InOldHeader = isExpansionInFileMatching(Spec.OldHeader);
-  auto InOldCC = isExpansionInFileMatching(Spec.OldCC);
+  auto InOldHeader = isExpansionInFile(
+      MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader));
+  auto InOldCC = isExpansionInFile(
+      MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC));
   auto InOldFiles = anyOf(InOldHeader, InOldCC);
   auto InMovedClass =
       hasDeclContext(cxxRecordDecl(hasName(FullyQualifiedName)));
@@ -279,22 +325,30 @@
   }
 }
 
-void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
-                                llvm::StringRef FileName) {
+void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader,
+                                llvm::StringRef SearchPath,
+                                bool IsAngled,
+                                llvm::StringRef FileName,
+                                const SourceManager& SM) {
   // 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 (!Spec.OldHeader.empty() &&
-      llvm::StringRef(Spec.OldHeader).endswith(IncludeHeader))
+  if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader) ==
+      MakeAbsolutePath(SearchPath, IncludeHeader))
     return;
 
   std::string IncludeLine =
       IsAngled ? ("#include <" + IncludeHeader + ">\n").str()
                : ("#include \"" + IncludeHeader + "\"\n").str();
-  if (!Spec.OldHeader.empty() && FileName.endswith(Spec.OldHeader))
+
+  std::string AbsolutePath = MakeAbsolutePath(SM, FileName);
+  if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader) ==
+      AbsolutePath) {
     HeaderIncludes.push_back(IncludeLine);
-  else if (!Spec.OldCC.empty() && FileName.endswith(Spec.OldCC))
+  } else if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC) ==
+             AbsolutePath) {
     CCIncludes.push_back(IncludeLine);
+  }
 }
 
 void ClangMoveTool::removeClassDefinitionInOldFiles() {
@@ -313,7 +367,8 @@
   std::vector<MovedDecl> NewHeaderDecls;
   std::vector<MovedDecl> NewCCDecls;
   for (const auto &MovedDecl : MovedDecls) {
-    if (IsInHeaderFile(*MovedDecl.SM, MovedDecl.Decl, Spec.OldHeader))
+    if (IsInHeaderFile(*MovedDecl.SM, MovedDecl.Decl, Spec.OldHeader,
+                       OriginalRunningDirectory))
       NewHeaderDecls.push_back(MovedDecl);
     else
       NewCCDecls.push_back(MovedDecl);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to