[PATCH] D65372: [llvm-objcopy] Add support for response files in llvm-strip and llvm-objcopy

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:414
+                     (ToolName + " tool").str().c_str());
+  OS << "\nPass @FILE as argument to read options from FILE.\n";
+}
----------------
This method is a bit of a hack, as the older library has a `cl::extrahelp` class that's usually used to support this [1], but libOption doesn't have yet. We should probably add one. Can you add a comment here to remove this once something like that is supported?

[1] e.g. https://github.com/llvm/llvm-project/blob/master/llvm/tools/llvm-size/llvm-size.cpp#L102


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:325-331
+  argv = const_cast<char **>(&NewArgv[0]);
+  argc = static_cast<int>(NewArgv.size());
+
   Expected<DriverConfig> DriverConfig =
-      IsStrip ? parseStripOptions(makeArrayRef(argv + 1, argc), reportWarning)
-              : parseObjcopyOptions(makeArrayRef(argv + 1, argc));
+      IsStrip
+          ? parseStripOptions(makeArrayRef(argv + 1, argc - 1), reportWarning)
+          : parseObjcopyOptions(makeArrayRef(argv + 1, argc - 1));
----------------
Instead of manipulating argc/argv directly, you can convert NewArgv directly to an array ref to pass to parse*Options, e.g.

```
  auto Args = makeArrayRef(NewArgv).drop_front();
  Expected<DriverConfig> DriverConfig =
      IsStrip ? parseStripOptions(Args, reportWarning)
              : parseObjcopyOptions(Args);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65372