[PATCH] D69665: [llvm-ar] Fix llvm-ar response file reading on Windows

[Owen Reynolds via Phabricator via llvm-commits]

gbreynoo marked an inline comment as done.
gbreynoo added inline comments.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1138
+  for (ArgVector::iterator ArgIt = Argv.begin(); ArgIt != Argv.end(); ++ArgIt) {
+    if (const char *Match =
+            matchFlagWithArg("rsp-quoting", ArgIt, Argv)) {
----------------
ruiu wrote:
> It seems that this code can be improved in terms of readability, like this:
> 
>   for (StringRef Arg : Argv) {
>     if (Arg == "-rsp-quoting=posix" || Arg == "--rsp-quoting=posix")
>       Ret = cl::TokenizeGNUCommandLine;
>     else if (Arg == "-rsp-quoting=windows" || Arg == "--rsp-quoting=windows")
>       Ret = cl::TokenizeWindowsCommandLine;
>   }
> 
> This is I believe basically what we do in the clang driver.
llvm-ar handles all the other long arguments with `MatchFlagWithArg` and the majority of this patch is so `rsp-quoting` can share this behaviour. Would you prefer to use the code above? It would reduce the size of the patch but would mean behaviour inconsistencies in how long arguments are handled.


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

https://reviews.llvm.org/D69665