Addressed all reviewer comments, Thanks.

Hi revane,

http://llvm-reviews.chandlerc.com/D648

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D648?vs=1574&id=1588#toc

Files:
  cpp11-migrate/Core/CMakeLists.txt
  cpp11-migrate/Core/IncludeExcludeInfo.cpp
  cpp11-migrate/Core/IncludeExcludeInfo.h
  cpp11-migrate/tool/Cpp11Migrate.cpp
  unittests/cpp11-migrate/CMakeLists.txt
  unittests/cpp11-migrate/IncludeExcludeTest.cpp
  unittests/cpp11-migrate/Makefile
Index: cpp11-migrate/Core/CMakeLists.txt
===================================================================
--- cpp11-migrate/Core/CMakeLists.txt
+++ cpp11-migrate/Core/CMakeLists.txt
@@ -3,6 +3,7 @@
 add_clang_library(migrateCore
   Transforms.cpp
   Transform.cpp
+  IncludeExcludeInfo.cpp
   )
 target_link_libraries(migrateCore
   clangTooling
Index: cpp11-migrate/Core/IncludeExcludeInfo.cpp
===================================================================
--- /dev/null
+++ cpp11-migrate/Core/IncludeExcludeInfo.cpp
@@ -0,0 +1,92 @@
+//===-- Core/IncludeExcludeInfo.cpp - IncludeExclude class impl -*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file provides the implemention of the IncludeExcludeInfo class
+/// to handle the include and exclude command line options.
+///
+//===----------------------------------------------------------------------===//
+
+#include "IncludeExcludeInfo.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+
+using namespace llvm;
+
+namespace {
+/// \brief Helper function to determine whether a file has the same path
+/// prefix as \a Path.
+///
+/// \a Path must be an absolute path.
+bool fileHasPathPrefix(StringRef File, StringRef Path) {
+  // Converts File to its absolute path.
+  SmallString<64> AbsoluteFile = File;
+  sys::fs::make_absolute(AbsoluteFile);
+
+  // Convert path strings to sys::path to iterate over each of its directories.
+  sys::path::const_iterator FileI = sys::path::begin(AbsoluteFile),
+                            FileE = sys::path::end(AbsoluteFile),
+                            PathI = sys::path::begin(Path),
+                            PathE = sys::path::end(Path);
+  while (FileI != FileE && PathI != PathE) {
+    // If the strings aren't equal then the two paths aren't contained within
+    // each other.
+    if (!FileI->equals(*PathI))
+      return false;
+    ++FileI;
+    ++PathI;
+  }
+  return true;
+}
+
+/// \brief Helper function to parse a string of comma seperated paths into
+/// the vector.
+void parseCLInput(StringRef Line, std::vector<std::string> &List) {
+  SmallVector<StringRef, 32> Tokens;
+  Line.split(Tokens, ",", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+  for (SmallVectorImpl<StringRef>::iterator I = Tokens.begin(),
+                                            E = Tokens.end();
+       I != E; ++I) {
+    // Convert each path to its absolute path.
+    SmallString<64> AbsolutePath = *I;
+    sys::fs::make_absolute(AbsolutePath);
+    List.push_back(std::string(AbsolutePath.str()));
+  }
+}
+} // end anonymous namespace
+
+IncludeExcludeInfo::IncludeExcludeInfo(StringRef Include, StringRef Exclude) {
+  parseCLInput(Include, IncludeList);
+  parseCLInput(Exclude, ExcludeList);
+}
+
+bool IncludeExcludeInfo::safeToTransform(StringRef FilePath) {
+  bool InIncludeList = false;
+
+  for (std::vector<std::string>::iterator I = IncludeList.begin(),
+                                          E = IncludeList.end();
+       I != E; ++I)
+    if ((InIncludeList = fileHasPathPrefix(FilePath, *I)))
+      break;
+  // If file is not in the list of included paths then it is not necessary
+  // to check the excluded path list.
+  if (!InIncludeList)
+    return false;
+
+  for (std::vector<std::string>::iterator I = ExcludeList.begin(),
+                                          E = ExcludeList.end();
+       I != E; ++I)
+    if (fileHasPathPrefix(FilePath, *I))
+      return false;
+
+  // If the file is in the included list but not in the excluded list, then
+  // it is safe to transform.
+  return true;
+}
Index: cpp11-migrate/Core/IncludeExcludeInfo.h
===================================================================
--- /dev/null
+++ cpp11-migrate/Core/IncludeExcludeInfo.h
@@ -0,0 +1,36 @@
+//===-- Core/IncludeExcludeInfo.h - IncludeExclude class def'n --*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file provides the definition for the IncludeExcludeInfo class
+/// to handle the include and exclude command line options.
+///
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_INCLUDEEXCLUDEINFO_H
+#define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_INCLUDEEXCLUDEINFO_H
+
+#include "llvm/ADT/StringRef.h"
+#include <vector>
+
+/// \brief Class encapsulating the handling of include and exclude paths
+/// provided by the user through command line options.
+class IncludeExcludeInfo {
+public:
+  IncludeExcludeInfo();
+  IncludeExcludeInfo(llvm::StringRef Include, llvm::StringRef Exclude);
+
+  /// \brief Determine if the given file is safe to transform.
+  bool safeToTransform(llvm::StringRef FilePath);
+
+private:
+  std::vector<std::string> IncludeList;
+  std::vector<std::string> ExcludeList;
+};
+
+#endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_INCLUDEEXCLUDEINFO_H
Index: cpp11-migrate/tool/Cpp11Migrate.cpp
===================================================================
--- cpp11-migrate/tool/Cpp11Migrate.cpp
+++ cpp11-migrate/tool/Cpp11Migrate.cpp
@@ -49,6 +49,17 @@
 SummaryMode("summary", cl::desc("Print transform summary"),
             cl::init(false));
 
+// TODO: Remove cl::Hidden when functionality for acknowledging include/exclude
+// options are implemented in the tool.
+static cl::opt<std::string>
+IncludePaths("include", cl::Hidden,
+             cl::desc("Comma seperated list of filepaths to consider to be "
+                      "transformed"));
+static cl::opt<std::string>
+ExcludePaths("exclude", cl::Hidden,
+             cl::desc("Comma seperated list of filepaths that can not "
+                      "be transformed"));
+
 class EndSyntaxArgumentsAdjuster : public ArgumentsAdjuster {
   CommandLineArguments Adjust(const CommandLineArguments &Args) {
     CommandLineArguments AdjustedArgs = Args;
Index: unittests/cpp11-migrate/CMakeLists.txt
===================================================================
--- unittests/cpp11-migrate/CMakeLists.txt
+++ unittests/cpp11-migrate/CMakeLists.txt
@@ -7,7 +7,8 @@
 include_directories(${CPP11_MIGRATE_SOURCE_DIR})
 
 add_extra_unittest(Cpp11MigrateTests
-  TransformTest.cpp)
+  TransformTest.cpp
+  IncludeExcludeTest.cpp)
 
 target_link_libraries(Cpp11MigrateTests
   migrateCore
Index: unittests/cpp11-migrate/IncludeExcludeTest.cpp
===================================================================
--- /dev/null
+++ unittests/cpp11-migrate/IncludeExcludeTest.cpp
@@ -0,0 +1,29 @@
+#include "Core/IncludeExcludeInfo.h"
+#include "gtest/gtest.h"
+
+IncludeExcludeInfo IEManager(/*include=*/ "a,b/b2,c/c2/c3",
+                             /*exclude=*/ "a/af.cpp,a/a2,b/b2/b2f.cpp,c/c2/c3");
+
+TEST(IncludeExcludeTest, NoMatchOnIncludeList) {
+  // If the file does not appear on the include list then it is not safe to
+  // transform. Files are not safe to transform by default.
+  EXPECT_FALSE(IEManager.safeToTransform("f.cpp"));
+  EXPECT_FALSE(IEManager.safeToTransform("b/dir/f.cpp"));
+}
+
+TEST(IncludeExcludeTest, MatchOnIncludeList) {
+  // If the file appears on only the include list then it is safe to transform.
+  EXPECT_TRUE(IEManager.safeToTransform("a/f.cpp"));
+  EXPECT_TRUE(IEManager.safeToTransform("a/dir/f.cpp"));
+  EXPECT_TRUE(IEManager.safeToTransform("b/b2/f.cpp"));
+}
+
+TEST(IncludeExcludeTest, MatchOnBothLists) {
+  // If the file appears on both the include or exclude list then it is not
+  // safe to transform.
+  EXPECT_FALSE(IEManager.safeToTransform("a/af.cpp"));
+  EXPECT_FALSE(IEManager.safeToTransform("a/a2/f.cpp"));
+  EXPECT_FALSE(IEManager.safeToTransform("a/a2/dir/f.cpp"));
+  EXPECT_FALSE(IEManager.safeToTransform("b/b2/b2f.cpp"));
+  EXPECT_FALSE(IEManager.safeToTransform("c/c2/c3/f.cpp"));
+}
Index: unittests/cpp11-migrate/Makefile
===================================================================
--- unittests/cpp11-migrate/Makefile
+++ unittests/cpp11-migrate/Makefile
@@ -16,7 +16,7 @@
 		   clangRewriteFrontend.a clangRewriteCore.a clangParse.a \
 		   clangSema.a clangAnalysis.a \
 		   clangAST.a clangASTMatchers.a clangEdit.a clangLex.a clangBasic.a \
-			 migrateCore.a
+		   migrateCore.a
 
 include $(CLANG_LEVEL)/Makefile
 MAKEFILE_UNITTEST_NO_INCLUDE_COMMON := 1
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to