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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/tools/llvm-ar/response.test:23
+## rsp-quoting
+# RUN: not llvm-ar --rsp-quoting=foobar @%t.response1.txt 2>&1 | \
+# RUN:   FileCheck  %s --check-prefix=ERROR
----------------
There is no positive `--rsp-quoting=` test, so it is hard to tell whether the feature works.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1102
 
+static StringRef removeHyphens(StringRef Arg) {
+  if (Arg.startswith("--"))
----------------
Unused


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1121
+
+  if (Arg.startswith("-"))
+    Arg = Arg.substr(1);
----------------
Three-dash `---rsp-quoting` will be accidentally accepted.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1137
+
+static cl::TokenizerCallback getRspQuoting(SmallVector<const char *, 0> Argv) {
+  cl::TokenizerCallback Ret =
----------------
`SmallVector` -> `ArrayRef`


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1165
+
+  for (SmallVector<const char *, 0>::iterator ArgIt = Argv.begin();
+       ArgIt != Argv.end(); ++ArgIt) {
----------------
I think we may need some refactoring first. Let me check if the parsing logic can be simplified.


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

https://reviews.llvm.org/D69665