[flang-commits] [PATCH] D124846: [flang][driver] Support reading response files

[Diana Picus via Phabricator via flang-commits]

rovka added inline comments.


================
Comment at: flang/tools/flang-driver/driver.cpp:79
 
+  // Parse response files. We're defaulting to the GNU syntax, since we don't
+  // have a CL mode.
----------------
awarzynski wrote:
> rovka wrote:
> > awarzynski wrote:
> > > 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.
> > Can do.
> 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.
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*.


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

https://reviews.llvm.org/D124846