[PATCH] Add support to read include/exclude path lists from file

[Jack Yang]

  Fixed issues and reimplemented interface as per Edwin's comments. Thanks.

  Regarding Windows/Mac compatibility:
  There should be no reason why it shouldn't work since we're using MemoryBuffer, an api that provides read-only access used to read from file and stdin.

  In MemoryBuffer.cpp, there's code to handle for os-dependent specifics. For example, MemoryBuffer::getFile uses "#ifdef LLVM_ON_WIN32" to handle for windows filepaths. It also uses llvm:sys which provides the os-dependent details.
  MemoryBuffer stores the file contents as a char* so there should be no reason why it would not work on other environments:
    const char *getBufferStart() const { return BufferStart; }
    const char *getBufferEnd() const   { return BufferEnd; }
    StringRef getBuffer() const {
      return StringRef(BufferStart, getBufferSize());
    }

Hi revane,

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D681?vs=1656&id=1670#toc

Files:
  cpp11-migrate/Core/IncludeExcludeInfo.cpp
  cpp11-migrate/Core/IncludeExcludeInfo.h
  cpp11-migrate/tool/Cpp11Migrate.cpp
  unittests/cpp11-migrate/IncludeExcludeTest.cpp

Index: cpp11-migrate/Core/IncludeExcludeInfo.cpp
===================================================================
--- cpp11-migrate/Core/IncludeExcludeInfo.cpp
+++ cpp11-migrate/Core/IncludeExcludeInfo.cpp
@@ -16,7 +16,9 @@
 #include "IncludeExcludeInfo.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
@@ -46,11 +48,12 @@
   return true;
 }
 
-/// \brief Helper function to parse a string of comma seperated paths into
+/// \brief Helper function to tokenize a string of filepaths and populate
 /// the vector.
-void parseCLInput(StringRef Line, std::vector<std::string> &List) {
+void parseCLInput(StringRef Line, std::vector<std::string> &List,
+                  StringRef Separator) {
   SmallVector<StringRef, 32> Tokens;
-  Line.split(Tokens, ",", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+  Line.split(Tokens, Separator, /*MaxSplit=*/ -1, /*KeepEmpty=*/ false);
   for (SmallVectorImpl<StringRef>::iterator I = Tokens.begin(),
                                             E = Tokens.end();
        I != E; ++I) {
@@ -62,9 +65,32 @@
 }
 } // end anonymous namespace
 
-IncludeExcludeInfo::IncludeExcludeInfo(StringRef Include, StringRef Exclude) {
-  parseCLInput(Include, IncludeList);
-  parseCLInput(Exclude, ExcludeList);
+error_code IncludeExcludeInfo::readListFromString(StringRef IncludeString,
+                                                  StringRef ExcludeString) {
+  parseCLInput(IncludeString, IncludeList, /*Separator=*/ ",");
+  parseCLInput(ExcludeString, ExcludeList, /*Separator=*/ ",");
+  return error_code::success();
+}
+
+error_code IncludeExcludeInfo::readListFromFile(StringRef IncludeListFile,
+                                                StringRef ExcludeListFile) {
+  if (!IncludeListFile.empty()) {
+    OwningPtr<MemoryBuffer> FileBuf;
+    if (error_code Err = MemoryBuffer::getFile(IncludeListFile, FileBuf)) {
+      errs() << "Unable to read from include file.\n";
+      return Err;
+    } else
+      parseCLInput(FileBuf->getBuffer(), IncludeList, /*Separator=*/ "\n");
+  }
+  if (!ExcludeListFile.empty()) {
+    OwningPtr<MemoryBuffer> FileBuf;
+    if (error_code Err = MemoryBuffer::getFile(ExcludeListFile, FileBuf)) {
+      errs() << "Unable to read from exclude file.\n";
+      return Err;
+    } else
+      parseCLInput(FileBuf->getBuffer(), ExcludeList, /*Separator=*/ "\n");
+  }
+  return error_code::success();
 }
 
 bool IncludeExcludeInfo::isFileIncluded(StringRef FilePath) {
Index: cpp11-migrate/Core/IncludeExcludeInfo.h
===================================================================
--- cpp11-migrate/Core/IncludeExcludeInfo.h
+++ cpp11-migrate/Core/IncludeExcludeInfo.h
@@ -16,16 +16,27 @@
 #define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_INCLUDEEXCLUDEINFO_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/system_error.h"
 #include <vector>
 
 /// \brief Class encapsulating the handling of include and exclude paths
 /// provided by the user through command line options.
 class IncludeExcludeInfo {
 public:
-  /// \brief Determine if the given file is safe to transform.
+  /// \brief Read and parse a comma-seperated lists of filepaths from
+  /// \a IncludeString and \a ExcludeString.
   ///
-  /// \a Include and \a Exclude must be formatted as a comma-seperated list.
-  IncludeExcludeInfo(llvm::StringRef Include, llvm::StringRef Exclude);
+  /// Returns error_code::success() on successful parse of the strings.
+  llvm::error_code readListFromString(llvm::StringRef IncludeString,
+                                      llvm::StringRef ExcludeString);
+
+  /// \brief Read and parse the lists of filepaths from \a IncludeListFile
+  /// and \a ExcludeListFile.
+  ///
+  /// Returns error_code::success() on successful read of both files or an
+  /// error_code indicating the encountered error.
+  llvm::error_code readListFromFile(llvm::StringRef IncludeListFile,
+                                    llvm::StringRef ExcludeListFile);
 
   /// \brief Determine if the given filepath is in the list of include paths but
   /// not in the list of exclude paths.
Index: cpp11-migrate/tool/Cpp11Migrate.cpp
===================================================================
--- cpp11-migrate/tool/Cpp11Migrate.cpp
+++ cpp11-migrate/tool/Cpp11Migrate.cpp
@@ -60,6 +60,14 @@
 ExcludePaths("exclude", cl::Hidden,
              cl::desc("Comma seperated list of filepaths that can not "
                       "be transformed"));
+static cl::opt<std::string>
+IncludeFromFile("include-from", cl::Hidden, cl::value_desc("filename"),
+                cl::desc("File containing a list of filepaths to consider to "
+                         "be transformed"));
+static cl::opt<std::string>
+ExcludeFromFile("exclude-from", cl::Hidden, cl::value_desc("filename"),
+                cl::desc("File containing a list of filepaths that can not be "
+                         "transforms"));
 
 class EndSyntaxArgumentsAdjuster : public ArgumentsAdjuster {
   CommandLineArguments Adjust(const CommandLineArguments &Args) {
Index: unittests/cpp11-migrate/IncludeExcludeTest.cpp
===================================================================
--- unittests/cpp11-migrate/IncludeExcludeTest.cpp
+++ unittests/cpp11-migrate/IncludeExcludeTest.cpp
@@ -1,24 +1,21 @@
 #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, FunctionalityTest) {
+  IncludeExcludeInfo IEManager;
+  IEManager.readListFromString(/*include=*/ "a,b/b2,c/c2",
+                               /*exclude=*/ "a/af.cpp,a/a2,b/b2/b2f.cpp,c/c2");
 
-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.isFileIncluded("f.cpp"));
   EXPECT_FALSE(IEManager.isFileIncluded("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.isFileIncluded("a/f.cpp"));
   EXPECT_TRUE(IEManager.isFileIncluded("a/dir/f.cpp"));
   EXPECT_TRUE(IEManager.isFileIncluded("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.isFileIncluded("a/af.cpp"));
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D681.2.patch
Type: text/x-patch
Size: 6668 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130417/bf698ed0/attachment.bin>