[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:1127
+
+static cl::TokenizerCallback getRspQuoting(ArgVector &Argv) {
+  cl::TokenizerCallback Default =
----------------
MaskRay wrote:
> Argv is passed as a non-const reference but it is not actually mutated in the function as I expected. Should getRspQuoting remove --rsp-quoting so that on L1204 we don't need `    if (matchFlagWithArg("rsp-quoting", Arg, ArgIt, Argv.end())...` ?
Good catch, it was left over from a previous implementation.  I think with the current implementation it may be best not to modify `Argv`. The call to `matchFlagWithArg` is cleaner than removing `rsp-quoting` arguments that can be in one vector element or in two elements. If there is an implementation you had in mind I'd be happy to make changes.





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

https://reviews.llvm.org/D69665