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

[Rafael Auler]

Hi Rafael,

I have answered your comments below, thanks. I will upload a patch fixing your concerns shortly after this message.

Sean, regarding the change to llvm::sys::argumentsFitWithinSystemLimits() to avoid code duplication, I am afraid it is not necessary after my new patch. I will send it now.

Regards,
Rafael

================
Comment at: include/clang/Driver/Job.h:87
@@ +86,3 @@
+  /// file, when it is used.
+  void PrintArgsWithRespFile(llvm::raw_ostream &OS,
+                             const char *Terminator, bool Quote,
----------------
rafael wrote:
> printArgsWithRespFile
changed

================
Comment at: include/clang/Driver/Job.h:100
@@ +99,3 @@
+  /// limits (the maximum command line length).
+  bool NeedsResponseFile() const { return needsResponseFile; }
+
----------------
rafael wrote:
> The function should be lowercase, the variable Uppercase.
changed

================
Comment at: lib/Driver/Job.cpp:32
@@ +31,3 @@
+      Executable(_Executable), Arguments(_Arguments),
+      needsResponseFile(
+          !llvm::sys::argumentsFitWithinSystemLimits(_Arguments) &&
----------------
rafael wrote:
> I wonder if we should say that the job needs a response file based only on llvm::sys::argumentsFitWithinSystemLimits and then emit an error if Creator.getResponseFilesSupport() says it is not supported.
The problem is that llvm::sys::argumentsFitWithinSystemLimits() is not exact, see:

bool llvm::sys::argumentsFitWithinSystemLimits(ArrayRef<const char*> Args) {
  static long ArgMax = sysconf(_SC_ARG_MAX);

  // System says no practical limit.
  if (ArgMax == -1)
    return true;

  // Conservatively account for space required by environment variables.
  ArgMax /= 2;

...
Thus, sometimes it may say that you need a response file but, when building the argv vector, you discover that it actually fits in system limits. Therefore, I think that our best action is to silently try to pass the arguments without a response file because it may actually work.

================
Comment at: lib/Driver/Job.cpp:279
@@ +278,3 @@
+  if (needsResponseFile && ResponseFile.data() != nullptr) {
+    PrintArgsWithRespFile(OS, Terminator, Quote, CrashReport);
+    return;
----------------
rafael wrote:
> The code in PrintArgsWithRespFile looks fairly duplicated with what just follows.  Couldn't this just patch Arguments and use the existing code path?
> 
Done.

http://reviews.llvm.org/D4897