[PATCH] D62753: [llvm-lipo] Implement -archs

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 31 15:46:07 PDT 2019


alexshap added inline comments.


================
Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:216
+LLVM_ATTRIBUTE_NORETURN
+static void printArchs(ArrayRef<OwningBinary<Binary>> InputBinaries) {
+  assert(InputBinaries.size() == 1 && "Incorrect number of input binaries");
----------------
smeenai wrote:
> compnerd wrote:
> > Why not just make the parameter an `OwningBinary<Binary>` rather than the `ArrayRef` and then assert that a single entry is provided?
> Good idea. We'd probably also wanna take the parameter by const reference?
not sure it really matters, the thing is that in general lipo can have multiple input files,
different actions expect different number of input files, thus since in main() we dispatch handling of particular commands to different subroutines  i think it's clear / consistent enough. I'd prefer to not to make that "top-level" code less readable, although it's simple either way so i don't insist.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62753





More information about the llvm-commits mailing list