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

Edwin Vane edwin.vane at intel.com
Wed Apr 17 06:59:19 PDT 2013


  I think now that we have two ways of providing the same data that we need to look again at the interface to IncludeExcludeInfo. Right now the constructor takes one set of data and a member function takes another which implies a difference in how the data is treated in some way. I think an interface where the data is treated equally is something to aim for. One example is to just provide a default constructor and then two member functions to provide data. This way the functions can return error codes. If all data is passed to the constructor then failures would need to be exceptions and LLVM doesn't allow those.


================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:64
@@ +63,3 @@
+static cl::opt<std::string>
+IncludeFromFile("include-from-file", cl::Hidden,
+                cl::desc("File containing a list of filepaths to consider to "
----------------
I'd suggest calling this 'include-from' and use cl::value_desc() to provide the name FILE. This would be in line with other tools like rsync.

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:69
@@ -65,3 +68,3 @@
 IncludeExcludeInfo::IncludeExcludeInfo(StringRef Include, StringRef Exclude) {
-  parseCLInput(Include, IncludeList);
-  parseCLInput(Exclude, ExcludeList);
+  parseCLInput(Include, IncludeList, ",");
+  parseCLInput(Exclude, ExcludeList, ",");
----------------
I'd suggest a /*Separator=*/ comment here.

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:54
@@ -52,1 +53,3 @@
+void parseCLInput(StringRef Line, std::vector<std::string> &List,
+                  StringRef Seperator) {
   SmallVector<StringRef, 32> Tokens;
----------------
Note the correct spelling of Separator.

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:82
@@ +81,3 @@
+    else
+      parseCLInput(FileBuf->getBuffer(), IncludeList, "\n");
+  }
----------------
Does this work on Windows/Mac?

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:78
@@ +77,3 @@
+    if (error_code Err = MemoryBuffer::getFile(IncludeListFile, FileBuf)) {
+      errs() << "Unable to read from exclude file.\n";
+      return Err;
----------------
exclude->include


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



More information about the cfe-commits mailing list