[flang][driver] Support reading response files

[rovka]

Summary

Add support for reading response files in the flang driver. Response
files contain command line arguments and are used whenever a command
becomes longer than the shell/environment limit. Response files are
recognized via the special "@path/to/response/file.rsp" syntax, which
distinguishes them from other file inputs.

This patch hardcodes GNU tokenization, since we don't have a CL mode for
the driver. In the future we might want to add a --rsp-quoting command
line option, like clang has, to accommodate Windows platforms.

[awarzynski]

Thanks for working on this! I would appreciate a note about the command line semantics for response files. IIUC (based on the tests file), one just needs to prepend the filename with @, but I couldn't find any documentation.

flang/test/Driver/response-file.f90

5

What happens if you skip @? Perhaps test for that too.

flang/tools/flang-driver/driver.cpp

91

Would it make sense to move this to a dedicated function? I imagine that this might grow in the future (especially when working on Windows). No strong opinion.

[rovka]

flang/test/Driver/response-file.f90

5

We get a 'linker input unused' warning (same as clang). Not sure if that's really worth testing...

flang/tools/flang-driver/driver.cpp

91

Can do.

[rovka]

• Extracted helper function
• Added a note about the @ syntax to the commit message (I hope that's what you had in mind, if not let me know!)
• Removed the comments about writing response files. It seems we already support that thanks to the common clang infrastructure and this line. Yay \o/

[awarzynski]

In D124846#3493556, @rovka wrote:

• Added a note about the @ syntax to the commit message (I hope that's what you had in mind, if not let me know!)

Your comment is perfect, thanks!

• Removed the comments about writing response files. It seems we already support that thanks to the common clang infrastructure and this line. Yay \o/

Haha, it's been there since before flang-new existed 😂 !

flang/test/Driver/response-file.f90

5

Perhaps the actual error is not relevant, but this would be nice:

! Works fine - valid syntax
! RUN: %flang -E -cpp @%basename_t.rsp %s -o - | FileCheck %s
! Fails - invalid syntax
! RUN: not %flang -E -cpp %basename_t.rsp %s -o - | FileCheck %s

flang/tools/flang-driver/driver.cpp

91

Thanks for the update! I was thinking about something that would hide all the implementation details of dealing with response files from main, e.g.:

ProcessResponseFiles(args);

Basically, somebody scanning main does not need to know/care about response files at all, right? The most important part is that args is being updated with the contents of response files.

Perhaps I'm a bit picky, but to me response files are still something exotic and niche and hence I'm keen to make this very clear :)

This is not a blocker for me.

[rovka]

flang/tools/flang-driver/driver.cpp

91

You mean move the BumpPtrAllocator and StringSaver inside the helper? That would be nice, but IIUC they have to be outside of it because they're keeping the storage for whatever new args are being added (and which we use in the rest of the code below). I think I could at most move the StringSaver inside the helper and pass the Allocator in instead, but this seems like a clearer interface to me *shrug*.

[rovka]

Add test without @.

[awarzynski]

Thanks for the updates, LGTM!

flang/test/Driver/response-file.f90

+! Test that the driver can process response files.
+
+! RUN: echo "-DTEST" > %basename_t.rsp
+! RUN: %flang -E -cpp @%basename_t.rsp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -E -cpp @%basename_t.rsp %s -o - | FileCheck %s
+! RUN: not %flang %basename_t.rsp %s -o /dev/null
+! RUN: not %flang_fc1 %basenamt_t.rsp %s -o /dev/null
+
+! CHECK-LABEL: program test
+! CHECK: end program
+
+#ifdef TEST
+program test
+end program
+#else
+We should have read the define from the response file.
+#endif

flang/tools/flang-driver/driver.cpp

 #include "flang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Driver/Compilation.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 using llvm::StringRef;
 
 // main frontend method. Lives inside fc1_main.cpp
 extern int fc1_main(llvm::ArrayRef<const char *> argv, const char *argv0);
static int executeFC1Tool(llvm::SmallVectorImpl<const char *> &argV) {
 
   // Reject unknown tools.
   // ATM it only supports fc1. Any fc1[*] is rejected.
   llvm::errs() << "error: unknown integrated tool '" << tool << "'. "
                << "Valid tools include '-fc1'.\n";
   return 1;
 }
 
+static void ExpandResponseFiles(
+    llvm::StringSaver &saver, llvm::SmallVectorImpl<const char *> &args) {
+  // We're defaulting to the GNU syntax, since we don't have a CL mode.
+  llvm::cl::TokenizerCallback tokenizer = &llvm::cl::TokenizeGNUCommandLine;
+  llvm::cl::ExpandResponseFiles(saver, tokenizer, args, /* MarkEOLs=*/false);
+}
+
 int main(int argc, const char **argv) {
 
   // Initialize variables to call the driver
   llvm::InitLLVM x(argc, argv);
   llvm::SmallVector<const char *, 256> args(argv, argv + argc);
 
   clang::driver::ParsedClangName targetandMode("flang", "--driver-mode=flang");
   std::string driverPath = getExecutablePath(args[0]);
 
+  llvm::BumpPtrAllocator a;
+  llvm::StringSaver saver(a);
+  ExpandResponseFiles(saver, args);
+
   // Check if flang-new is in the frontend mode
   auto firstArg = std::find_if(
       args.begin() + 1, args.end(), [](const char *a) { return a != nullptr; });
   if (firstArg != args.end()) {
     if (llvm::StringRef(args[1]).startswith("-cc1")) {
       llvm::errs() << "error: unknown integrated tool '" << args[1] << "'. "
                    << "Valid tools include '-fc1'.\n";
       return 1;