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

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

alexshap added inline comments.

Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:216
+static void printArchs(ArrayRef<OwningBinary<Binary>> InputBinaries) {
+  assert(InputBinaries.size() == 1 && "Incorrect number of input binaries");
alexshap wrote:
> 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.
discussed with @compnerd and we agreed on this, might be worth adding a comment in the code (in main) that the details/specifics of handling particular options (i.e. the number of expected input files) are inside the corresponding subroutines.

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list