[PATCH] Teach Clang how to use response files when calling other tools

[Sean Silva]

I'm still uneasy about NeedsResponseFile (see my comment below).

Also a few style things.

================
Comment at: include/clang/Driver/Job.h:101-104
@@ +100,6 @@
+
+  /// Returns whether we should use a response file when executing this command
+  bool useResponseFile() const {
+    return ResponseFile != nullptr && NeedsResponseFile;
+  }
+
----------------
I still don't think this makes sense. You previously replied that this is done to "protect the user", but I think that two combinations are programming errors: We should assert if:
ResponseFile == nullptr and NeedsResponseFile == true
ResponseFile != nullptr and NeedsResponseFile == false
(if this is not the case, please provide an example where this could happen and clang would behave correctly)

This leaves only two valid options:
ResponseFile == nullptr and NeedsResponseFile == false
ResponseFile != nullptr and NeedsResponseFile == true

So NeedsResponseFile is redundant.

It seems like right now you are using NeedsResponseFile just as a flag that roughly means "remember to do some other work at some faraway place in the code". I really don't like having such a random coupling across disparate parts of the code. Can you centralize the logic of checking `!llvm::sys::argumentsFitWithinSystemLimits(_Arguments) && Creator.getResponseFilesSupport() != Tool::RF_None` with the logic of setting ResponseFile?

================
Comment at: lib/Driver/Job.cpp:120-122
@@ +119,5 @@
+  for (const char *Arg : Arguments) {
+    bool NeedsQuoting = ArgNeedsQuotes(Arg);
+    if (NeedsQuoting)
+      OS << '"';
+
----------------
Can we simplify this by just always quoting?

================
Comment at: lib/Driver/Job.cpp:124-129
@@ +123,8 @@
+
+    while (*Arg != '\0') {
+      if (*Arg == '\"' || *Arg == '\\') {
+        OS << '\\';
+      }
+      OS << *Arg++;
+    }
+
----------------
Please make this a for loop so that it is easy to see what is being looped over. In particular, so that the Arg++ is easily visible.

for (; *Arg != '\0'; Arg++)

================
Comment at: lib/Driver/Tools.cpp:5854-5865
@@ +5853,14 @@
+  // need it later
+  for (const auto &II : Inputs) {
+    if (II.isFilename()) {
+      InputFileList.push_back(II.getFilename());
+      continue;
+    }
+
+    // This is a linker input argument.
+    // We cannot mix input arguments and file names in a -filelist input, thus
+    // we prematurely stop our list (remaining files shall be passed as
+    // arguments).
+    break;
+  }
+
----------------
Simpler:

  for (const auto &II : Inputs) {
    if (!II.isFilename())
      break;
    InputFileList.push_back(II.getFilename());
  }

http://reviews.llvm.org/D4897