[PATCH] D62798: Explicitly detect recursive response files

[James Y Knight via Phabricator via llvm-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

I like it. Some minor suggestions for a few extra comments, but other than that LGTM.



================
Comment at: llvm/lib/Support/CommandLine.cpp:1106
+
+  SmallVector<ResponseFileRecord, 3> FileStack;
+
----------------
Could use a comment as to what this represents -- the position of the last argument that came (including recursively) from the named file.


================
Comment at: llvm/lib/Support/CommandLine.cpp:1114
   for (unsigned I = 0; I != Argv.size();) {
+    while (I == FileStack.back().End) {
+      FileStack.pop_back();
----------------
Also can use a comment -- drop any files from the stack if we've finished their arguments.


================
Comment at: llvm/lib/Support/CommandLine.cpp:1167
+
+  assert(Argv.size() == FileStack.back().End);
   return AllExpanded;
----------------
Worth putting a comment here as to why this is the case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62798/new/

https://reviews.llvm.org/D62798