[PATCH] D64668: [llvm-lipo] Implement -info

[Anusha Basana via Phabricator via llvm-commits]

anushabasana marked an inline comment as not done.
anushabasana added inline comments.


================
Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:301
+static void printInfo(ArrayRef<OwningBinary<Binary>> InputBinaries) {
+  std::string UniversalOutputBuffer;
+  raw_string_ostream UniversalOutputStream(UniversalOutputBuffer);
----------------
alexshap wrote:
> alexshap wrote:
> > khm, the same effect (grouping universal/regular files) could be achieved a bit simpler, what would you say to the following (no need for std::string, raw_string_ostream, ThinOutputBuffer, ThinOutputStream, UniversalOutputBuffer, UniversalOutputStream, etc)
> >      
> >     for (const auto &IB : InputBinaries)
> >         if (IB.getBinary()->isMachOUniversalBinary()) {
> >              outs() << "Architectures in the fat file: " << Binary->getFileName() 
> >                        << " are: ";
> >              printBinaryArchs(IB.getBinary(), outs());
> >         }
> >      for (const auto &IB : InputBinaries)
> >         if (IB.getBinary()->isMachO) {
> >              outs() << "Non-fat file: " << IB.getBinary()->getFileName()
> >                        << " is architecture: ";
> >              printBinaryArchs(IB.getBinary(), outs());
> >         }            
> cc: @mtrent 
> 1. The wording (especially in the second case) appears to be very weird, maybe it's worth to change it and make it more human-readable ? (I suspect `-info` is not really widely used in scripts, so probably this incompatibility might not be a big issue).
> 2. Maybe we don't even need to group anything here (for the same reasons)
I could remove the string and OS for Universal files and just pass in outs() to cut down there.
I originally had 2 for loops but I thought the multiple OS would be better so we would only have to do one pass over the input binaries.
I'm not sure what is more legible, I don't have a strong preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64668