[PATCH] Implement handling of include/exclude options for cpp11-migrate

Manuel Klimek klimek at google.com
Wed Apr 10 00:35:43 PDT 2013



================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:23
@@ +22,3 @@
+
+static cl::opt<std::string> CLIncludePaths(
+    "include", cl::desc("Comma seperated list of filepaths to consider to be "
----------------
Just FYI: I personally have a strong dislike of putting command line arguments into library files - it means that every binary that happens to link in this file transitively will see this argument, which might not be a problem now, but in my experience it always leads to headaches down the road.
What I'd do instead is: put the command line args into the main file, and have a constructor for IncludeExcludeInfo that takes the lists as arguments.

================
Comment at: cpp11-migrate/Core/IncludeExcludeInfo.cpp:60
@@ +59,3 @@
+  SmallVector<StringRef, 32> Tokens;
+  Line.split(Tokens, ",", -1, false);
+  for (SmallVectorImpl<StringRef>::iterator I = Tokens.begin(),
----------------
I'd insert comments for the literal parameters like so:
Line.split(Tokens, ",", /*MaxSplit=*/-1, /*KeepEmpty=*/false);


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



More information about the cfe-commits mailing list