[PATCH] D87212: [llvm-objcopy][MachO] Add llvm-bitcode-strip driver

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:1013-1025
+  SmallVector<StringRef, 2> Positional;
+  for (auto Arg : InputArgs.filtered(BITCODE_STRIP_INPUT))
+    Positional.push_back(Arg->getValue());
+  // TODO: Add tests for these errors once we implement support for the
+  // tool-specific command line options. At the moment we do not reach these
+  // cases because of early returns.
+  if (Positional.empty())
----------------
jhenderson wrote:
> Same comment as above. I'd avoid adding code that can't currently be hit, and wait to add it until the functionality is required.
another option is to call "exit(0)" - without these checks InputfFilename/OutputFilename can't be safely set, and without them one can not construct a valid instance of Config.  Not sure if exit(0) inside the function which parses command-line options is better, but it doesn't matter much anyway. What's your preference ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87212