[PATCH] [Support] Beef up and expose the response file parsing in llvm::cl

[Reid Kleckner]

================
Comment at: include/llvm/Support/CommandLine.h:1787
@@ +1786,3 @@
+typedef void TokenizerCallback(StringRef Source, StringSaver &Saver,
+                               SmallVectorImpl<const char *> &NewArgv);
+
----------------
Hans Wennborg wrote:
> I think it's more common to typedef a callback type as pointer-to-function instead of just function.
True.  This is what happens after you work on a C++ compiler, you learn all kind of obscure features that no one else knows how to use.

================
Comment at: include/llvm/Support/CommandLine.h:1796
@@ +1795,3 @@
+/// \param [in,out] Argv Command line into which to expand response files.
+void ExpandResponseFiles(StringSaver &Saver, TokenizerCallback *Tokenize,
+                         SmallVectorImpl<const char *> &Argv);
----------------
Hans Wennborg wrote:
> if TokenizerCallback is typedeffed to ptr-to-function, Tokenize would be passed by value (well, the value would be a pointer), which I think is more common for callbacks. Also, maybe rename the parameter to "Tokenizer"?
Done.

================
Comment at: lib/Support/CommandLine.cpp:453
@@ +452,3 @@
+                                SmallVectorImpl<const char *> &NewArgv) {
+  SmallString<128> Tok;
+  for (size_t I = 0, E = Src.size(); I != E; ++I) {
----------------
Hans Wennborg wrote:
> no big deal, but we might as well call this Token?
Done.  I guess Tok is common in clang because Token is the class name.

================
Comment at: lib/Support/CommandLine.cpp:465
@@ +464,3 @@
+    if (I + 1 < E && Src[I] == '\\' && isGNUSpecial(Src[I + 1])) {
+      ++I;  // Consume the escape.
+      Tok.push_back(Src[I]);
----------------
Hans Wennborg wrote:
> isn't it more "skip" than consume?
Sure.

================
Comment at: lib/Support/CommandLine.cpp:538
@@ +537,3 @@
+    const char *Arg = Argv[I];
+    if (Arg[0] != '@' || RspFiles > 20) {
+      ++I;
----------------
Hans Wennborg wrote:
> should we just break out of the loop when RspFiles hits 20?
Yes, that actually works now.  Previously I was copying from OrigArgv to NewArgv and needed to continue the loop.

================
Comment at: unittests/Support/CommandLineTest.cpp:129
@@ +128,3 @@
+TEST(CommandLineTest, TokenizeGNUCommandLine) {
+  const char *Input = "foo\\ bar \"foo bar\" \'foo bar\' 'foo\\\\bar'";
+  const char *const Output[] = { "foo bar", "foo bar", "foo bar", "foo\\bar" };
----------------
Hans Wennborg wrote:
> Hans Wennborg wrote:
> > maybe add a test to check that a token doesn't have to end after a quote, i.e. foo"bar"baz is just one token: foobarbaz
> and maybe stick a windows file path in there to make sure we don't do anything crazy with the backslashes
Sounds good, both done.


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