[PATCH] D64102: [llvm-lipo] Implement -create part 1

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:274
   if (auto UO = dyn_cast<MachOUniversalBinary>(InputBinary)) {
-    for (MachOUniversalBinary::object_iterator I = UO->begin_objects(),
-                                               E = UO->end_objects();
-         I != E; ++I) {
+    for (auto I = UO->begin_objects(), E = UO->end_objects(); I != E; ++I) {
       Expected<std::unique_ptr<MachOObjectFile>> BinaryOrError =
----------------
I think that this can be replaced with a range based loop:

```
  for (auto O : UO->objects()) {
    Expected<std::unique_ptr<MachOObjectFile>> BinaryOrError = O.getAsObjectFile();
    if (!BinaryOrError)
      reportError(InputBinary->getFileName(), BinaryOrError.takeError());
    outs() << getArchString(BinaryOrError.get()->get()) << “ “;
  }
```


================
Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:329
 
+static void checkArchDuplicates(const ArrayRef<Slice> &Slices) {
+  DenseMap<uint64_t, const MachOObjectFile *> CPUIds;
----------------
I think that having a helper will help:

```
auto CPUIDForSlice = [](const Slice &S) -> uint64_t {
  return S.ObjectFile->getHeader().cputype << 32 | S.ObjectFile->getHeader().cpusubtype;
};
```


================
Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:404
+template <class FatArchType>
+static SmallVector<FatArchType, 2> buildFatArchList(ArrayRef<Slice> Slices) {
+  SmallVector<FatArchType, 2> FatArchList;
----------------
I think that it is better to return a `SmallVectorImpl<FatArchType>`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64102