Object: Replace NewArchiveIterator with a simpler NewArchiveMember class. NFCI.

[pcc]

Summary

The NewArchiveIterator class has a problem: it requires too much context. Any
memory buffers added to the archive must be stored within an Archive::Member,
which must have an associated Archive. This makes it harder than necessary
to create new archive members (or new archives entirely) from scratch using
memory buffers.

This patch replaces NewArchiveIterator with a NewArchiveMember class that
stores just the memory buffer and the information that goes into the archive
member header.

[• rafael]

I like the general idea.

include/llvm/Object/ArchiveWriter.h

25 ↗

(On Diff #61881)

Getting the time in here seems really odd.

32 ↗

(On Diff #61881)

Passing &EC to constructor is a really nasty thing. How about a private constructor and a static method that returns an Expected?

[pcc]

• Improved error handling

[pcc]

include/llvm/Object/ArchiveWriter.h

25 ↗

(On Diff #61881)

Hm? This is the (constant) epoch, not the current time.

[• rafael]

LGTM

llvm/trunk/include/llvm/Object/ArchiveWriter.h

 #define LLVM_OBJECT_ARCHIVEWRITER_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Support/FileSystem.h"
 
 namespace llvm {
 
-class NewArchiveIterator {
-  bool IsNewMember;
-  StringRef Name;
-
-  object::Archive::Child OldMember;
-
-public:
-  NewArchiveIterator(const object::Archive::Child &OldMember, StringRef Name);
-  NewArchiveIterator(StringRef FileName);
-  bool isNewMember() const;
-  StringRef getName() const;
-
-  const object::Archive::Child &getOld() const;
-
-  StringRef getNew() const;
-  llvm::ErrorOr<int> getFD(sys::fs::file_status &NewStatus) const;
-  const sys::fs::file_status &getStatus() const;
+struct NewArchiveMember {
+  std::unique_ptr<MemoryBuffer> Buf;
+  sys::TimeValue ModTime = sys::TimeValue::PosixZeroTime();
+  unsigned UID = 0, GID = 0, Perms = 0644;
+
+  NewArchiveMember() = default;
+  NewArchiveMember(MemoryBufferRef BufRef);
+
+  static Expected<NewArchiveMember>
+  getOldMember(const object::Archive::Child &OldMember, bool Deterministic);
+
+  static Expected<NewArchiveMember> getFile(StringRef FileName,
+                                            bool Deterministic);
 };
 
 std::pair<StringRef, std::error_code>
-writeArchive(StringRef ArcName, std::vector<NewArchiveIterator> &NewMembers,
+writeArchive(StringRef ArcName, std::vector<NewArchiveMember> &NewMembers,
              bool WriteSymtab, object::Archive::Kind Kind, bool Deterministic,
              bool Thin, std::unique_ptr<MemoryBuffer> OldArchiveBuf = nullptr);
 }
 
 #endif

llvm/trunk/lib/LibDriver/LibDriver.cpp

 class LibOptTable : public llvm::opt::OptTable {
 public:
   LibOptTable() : OptTable(infoTable, true) {}
 };
 
 }
 
 static std::string getOutputPath(llvm::opt::InputArgList *Args,
-                                 const llvm::NewArchiveIterator &FirstMember) {
+                                 const llvm::NewArchiveMember &FirstMember) {
   if (auto *Arg = Args->getLastArg(OPT_out))
     return Arg->getValue();
-  SmallString<128> Val = FirstMember.getNew();
+  SmallString<128> Val = StringRef(FirstMember.Buf->getBufferIdentifier());
   llvm::sys::path::replace_extension(Val, ".lib");
   return Val.str();
 }
 
 static std::vector<StringRef> getSearchPaths(llvm::opt::InputArgList *Args,
                                              StringSaver &Saver) {
   std::vector<StringRef> Ret;
   // Add current directory as first item of the search path.
int llvm::libDriverMain(llvm::ArrayRef<const char*> ArgsArr) {
 
   if (Args.filtered_begin(OPT_INPUT) == Args.filtered_end()) {
     // No input files.  To match lib.exe, silently do nothing.
     return 0;
   }
 
   std::vector<StringRef> SearchPaths = getSearchPaths(&Args, Saver);
 
-  std::vector<llvm::NewArchiveIterator> Members;
+  std::vector<llvm::NewArchiveMember> Members;
   for (auto *Arg : Args.filtered(OPT_INPUT)) {
     Optional<std::string> Path = findInputFile(Arg->getValue(), SearchPaths);
     if (!Path.hasValue()) {
       llvm::errs() << Arg->getValue() << ": no such file or directory\n";
       return 1;
     }
-    Members.emplace_back(Saver.save(*Path));
+    Expected<NewArchiveMember> MOrErr =
+        NewArchiveMember::getFile(Saver.save(*Path), /*Deterministic=*/true);
+    if (!MOrErr) {
+      handleAllErrors(MOrErr.takeError(), [&](const llvm::ErrorInfoBase &EIB) {
+        llvm::errs() << Arg->getValue() << ": " << EIB.message() << "\n";
+      });
+      return 1;
+    }
+    Members.emplace_back(std::move(*MOrErr));
   }
 
   std::pair<StringRef, std::error_code> Result =
       llvm::writeArchive(getOutputPath(&Args, Members[0]), Members,
                          /*WriteSymtab=*/true, object::Archive::K_GNU,
                          /*Deterministic*/ true, Args.hasArg(OPT_llvmlibthin));
 
   if (Result.second) {
     if (Result.first.empty())
       Result.first = ArgsArr[0];
     llvm::errs() << Result.first << ": " << Result.second.message() << "\n";
     return 1;
   }
 
   return 0;
 }

llvm/trunk/lib/Object/ArchiveWriter.cpp

 #if !defined(_MSC_VER) && !defined(__MINGW32__)
 #include <unistd.h>
 #else
 #include <io.h>
 #endif
 
 using namespace llvm;
 
-NewArchiveIterator::NewArchiveIterator(const object::Archive::Child &OldMember,
-                                       StringRef Name)
-    : IsNewMember(false), Name(Name), OldMember(OldMember) {}
+NewArchiveMember::NewArchiveMember(MemoryBufferRef BufRef)
+    : Buf(MemoryBuffer::getMemBuffer(BufRef, false)) {}
 
-NewArchiveIterator::NewArchiveIterator(StringRef FileName)
-    : IsNewMember(true), Name(FileName), OldMember(nullptr, nullptr, nullptr) {}
+Expected<NewArchiveMember>
+NewArchiveMember::getOldMember(const object::Archive::Child &OldMember,
+                               bool Deterministic) {
+  ErrorOr<llvm::MemoryBufferRef> BufOrErr = OldMember.getMemoryBufferRef();
+  if (!BufOrErr)
+    return errorCodeToError(BufOrErr.getError());
+
+  NewArchiveMember M;
+  M.Buf = MemoryBuffer::getMemBuffer(*BufOrErr, false);
+  if (!Deterministic) {
+    M.ModTime = OldMember.getLastModified();
+    M.UID = OldMember.getUID();
+    M.GID = OldMember.getGID();
+    M.Perms = OldMember.getAccessMode();
+  }
+  return std::move(M);
+}
+
+Expected<NewArchiveMember> NewArchiveMember::getFile(StringRef FileName,
+                                                     bool Deterministic) {
+  sys::fs::file_status Status;
+  int FD;
+  if (auto EC = sys::fs::openFileForRead(FileName, FD))
+    return errorCodeToError(EC);
+  assert(FD != -1);
 
-StringRef NewArchiveIterator::getName() const { return Name; }
-
-bool NewArchiveIterator::isNewMember() const { return IsNewMember; }
-
-const object::Archive::Child &NewArchiveIterator::getOld() const {
-  assert(!IsNewMember);
-  return OldMember;
-}
-
-StringRef NewArchiveIterator::getNew() const {
-  assert(IsNewMember);
-  return Name;
-}
-
-llvm::ErrorOr<int>
-NewArchiveIterator::getFD(sys::fs::file_status &NewStatus) const {
-  assert(IsNewMember);
-  int NewFD;
-  if (auto EC = sys::fs::openFileForRead(Name, NewFD))
-    return EC;
-  assert(NewFD != -1);
-
-  if (auto EC = sys::fs::status(NewFD, NewStatus))
-    return EC;
+  if (auto EC = sys::fs::status(FD, Status))
+    return errorCodeToError(EC);
 
   // Opening a directory doesn't make sense. Let it fail.
   // Linux cannot open directories with open(2), although
   // cygwin and *bsd can.
-  if (NewStatus.type() == sys::fs::file_type::directory_file)
-    return make_error_code(errc::is_a_directory);
+  if (Status.type() == sys::fs::file_type::directory_file)
+    return errorCodeToError(make_error_code(errc::is_a_directory));
 
-  return NewFD;
+  ErrorOr<std::unique_ptr<MemoryBuffer>> MemberBufferOrErr =
+      MemoryBuffer::getOpenFile(FD, FileName, Status.getSize(), false);
+  if (!MemberBufferOrErr)
+    return errorCodeToError(MemberBufferOrErr.getError());
+
+  if (close(FD) != 0)
+    return errorCodeToError(std::error_code(errno, std::generic_category()));
+
+  NewArchiveMember M;
+  M.Buf = std::move(*MemberBufferOrErr);
+  if (!Deterministic) {
+    M.ModTime = Status.getLastModificationTime();
+    M.UID = Status.getUser();
+    M.GID = Status.getGroup();
+    M.Perms = Status.permissions();
+  }
+  return std::move(M);
 }
 
 template <typename T>
 static void printWithSpacePadding(raw_fd_ostream &OS, T Data, unsigned Size,
                                   bool MayTruncate = false) {
   uint64_t OldPos = OS.tell();
   OS << Data;
   unsigned SizeSoFar = OS.tell() - OldPos;
static std::string computeRelativePath(StringRef From, StringRef To) {
 
   for (auto ToE = sys::path::end(To); ToI != ToE; ++ToI)
     sys::path::append(Relative, *ToI);
 
   return Relative.str();
 }
 
 static void writeStringTable(raw_fd_ostream &Out, StringRef ArcName,
-                             ArrayRef<NewArchiveIterator> Members,
+                             ArrayRef<NewArchiveMember> Members,
                              std::vector<unsigned> &StringMapIndexes,
                              bool Thin) {
   unsigned StartOffset = 0;
-  for (const NewArchiveIterator &I : Members) {
-    StringRef Name = sys::path::filename(I.getName());
+  for (const NewArchiveMember &M : Members) {
+    StringRef Path = M.Buf->getBufferIdentifier();
+    StringRef Name = sys::path::filename(Path);
     if (!useStringTable(Thin, Name))
       continue;
     if (StartOffset == 0) {
       printWithSpacePadding(Out, "//", 58);
       Out << "`\n";
       StartOffset = Out.tell();
     }
     StringMapIndexes.push_back(Out.tell() - StartOffset);
 
     if (Thin)
-      Out << computeRelativePath(ArcName, I.getName());
+      Out << computeRelativePath(ArcName, Path);
     else
       Out << Name;
 
     Out << "/\n";
   }
   if (StartOffset == 0)
     return;
   if (Out.tell() % 2)
static sys::TimeValue now(bool Deterministic) {
   sys::TimeValue TV;
   TV.fromEpochTime(0);
   return TV;
 }
 
 // Returns the offset of the first reference to a member offset.
 static ErrorOr<unsigned>
 writeSymbolTable(raw_fd_ostream &Out, object::Archive::Kind Kind,
-                 ArrayRef<NewArchiveIterator> Members,
-                 ArrayRef<MemoryBufferRef> Buffers,
+                 ArrayRef<NewArchiveMember> Members,
                  std::vector<unsigned> &MemberOffsetRefs, bool Deterministic) {
   unsigned HeaderStartOffset = 0;
   unsigned BodyStartOffset = 0;
   SmallString<128> NameBuf;
   raw_svector_ostream NameOS(NameBuf);
   LLVMContext Context;
   for (unsigned MemberNum = 0, N = Members.size(); MemberNum < N; ++MemberNum) {
-    MemoryBufferRef MemberBuffer = Buffers[MemberNum];
+    MemoryBufferRef MemberBuffer = Members[MemberNum].Buf->getMemBufferRef();
     Expected<std::unique_ptr<object::SymbolicFile>> ObjOrErr =
         object::SymbolicFile::createSymbolicFile(
             MemberBuffer, sys::fs::file_magic::unknown, &Context);
     if (!ObjOrErr) {
       // FIXME: check only for "not an object file" errors.
       consumeError(ObjOrErr.takeError());
       continue;
     }
else
     print32(Out, Kind, NumSyms * 8);
 
   Out.seek(Pos);
   return BodyStartOffset + 4;
 }
 
 std::pair<StringRef, std::error_code>
 llvm::writeArchive(StringRef ArcName,
-                   std::vector<NewArchiveIterator> &NewMembers,
+                   std::vector<NewArchiveMember> &NewMembers,
                    bool WriteSymtab, object::Archive::Kind Kind,
                    bool Deterministic, bool Thin,
                    std::unique_ptr<MemoryBuffer> OldArchiveBuf) {
   assert((!Thin || Kind == object::Archive::K_GNU) &&
          "Only the gnu format has a thin mode");
   SmallString<128> TmpArchive;
   int TmpArchiveFD;
   if (auto EC = sys::fs::createUniqueFile(ArcName + ".temp-archive-%%%%%%%.a",
                                           TmpArchiveFD, TmpArchive))
     return std::make_pair(ArcName, EC);
 
   tool_output_file Output(TmpArchive, TmpArchiveFD);
   raw_fd_ostream &Out = Output.os();
   if (Thin)
     Out << "!<thin>\n";
   else
     Out << "!<arch>\n";
 
   std::vector<unsigned> MemberOffsetRefs;
 
   std::vector<std::unique_ptr<MemoryBuffer>> Buffers;
   std::vector<MemoryBufferRef> Members;
   std::vector<sys::fs::file_status> NewMemberStatus;
 
-  for (NewArchiveIterator &Member : NewMembers) {
-    MemoryBufferRef MemberRef;
-
-    if (Member.isNewMember()) {
-      StringRef Filename = Member.getNew();
-      NewMemberStatus.resize(NewMemberStatus.size() + 1);
-      sys::fs::file_status &Status = NewMemberStatus.back();
-      ErrorOr<int> FD = Member.getFD(Status);
-      if (auto EC = FD.getError())
-        return std::make_pair(Filename, EC);
-      ErrorOr<std::unique_ptr<MemoryBuffer>> MemberBufferOrErr =
-          MemoryBuffer::getOpenFile(FD.get(), Filename, Status.getSize(),
-                                    false);
-      if (auto EC = MemberBufferOrErr.getError())
-        return std::make_pair(Filename, EC);
-      if (close(FD.get()) != 0)
-        return std::make_pair(Filename,
-                              std::error_code(errno, std::generic_category()));
-      Buffers.push_back(std::move(MemberBufferOrErr.get()));
-      MemberRef = Buffers.back()->getMemBufferRef();
-    } else {
-      const object::Archive::Child &OldMember = Member.getOld();
-      assert((!Thin || OldMember.getParent()->isThin()) &&
-             "Thin archives cannot refers to member of other archives");
-      ErrorOr<MemoryBufferRef> MemberBufferOrErr =
-          OldMember.getMemoryBufferRef();
-      if (auto EC = MemberBufferOrErr.getError())
-        return std::make_pair("", EC);
-      MemberRef = MemberBufferOrErr.get();
-    }
-    Members.push_back(MemberRef);
-  }
-
   unsigned MemberReferenceOffset = 0;
   if (WriteSymtab) {
     ErrorOr<unsigned> MemberReferenceOffsetOrErr = writeSymbolTable(
-        Out, Kind, NewMembers, Members, MemberOffsetRefs, Deterministic);
+        Out, Kind, NewMembers, MemberOffsetRefs, Deterministic);
     if (auto EC = MemberReferenceOffsetOrErr.getError())
       return std::make_pair(ArcName, EC);
     MemberReferenceOffset = MemberReferenceOffsetOrErr.get();
   }
 
   std::vector<unsigned> StringMapIndexes;
   if (Kind != object::Archive::K_BSD)
     writeStringTable(Out, ArcName, NewMembers, StringMapIndexes, Thin);
 
-  unsigned MemberNum = 0;
-  unsigned NewMemberNum = 0;
   std::vector<unsigned>::iterator StringMapIndexIter = StringMapIndexes.begin();
   std::vector<unsigned> MemberOffset;
-  for (const NewArchiveIterator &I : NewMembers) {
-    MemoryBufferRef File = Members[MemberNum++];
+  for (const NewArchiveMember &M : NewMembers) {
+    MemoryBufferRef File = M.Buf->getMemBufferRef();
 
     unsigned Pos = Out.tell();
     MemberOffset.push_back(Pos);
 
-    sys::TimeValue ModTime;
-    unsigned UID;
-    unsigned GID;
-    unsigned Perms;
-    if (Deterministic) {
-      ModTime.fromEpochTime(0);
-      UID = 0;
-      GID = 0;
-      Perms = 0644;
-    } else if (I.isNewMember()) {
-      const sys::fs::file_status &Status = NewMemberStatus[NewMemberNum];
-      ModTime = Status.getLastModificationTime();
-      UID = Status.getUser();
-      GID = Status.getGroup();
-      Perms = Status.permissions();
-    } else {
-      const object::Archive::Child &OldMember = I.getOld();
-      ModTime = OldMember.getLastModified();
-      UID = OldMember.getUID();
-      GID = OldMember.getGID();
-      Perms = OldMember.getAccessMode();
-    }
-
-    if (I.isNewMember()) {
-      StringRef FileName = I.getNew();
-      const sys::fs::file_status &Status = NewMemberStatus[NewMemberNum++];
-      printMemberHeader(Out, Kind, Thin, sys::path::filename(FileName),
-                        StringMapIndexIter, ModTime, UID, GID, Perms,
-                        Status.getSize());
-    } else {
-      const object::Archive::Child &OldMember = I.getOld();
-      ErrorOr<uint32_t> Size = OldMember.getSize();
-      if (std::error_code EC = Size.getError())
-        return std::make_pair("", EC);
-      StringRef FileName = I.getName();
-      printMemberHeader(Out, Kind, Thin, sys::path::filename(FileName),
-                        StringMapIndexIter, ModTime, UID, GID, Perms,
-                        Size.get());
-    }
+    printMemberHeader(Out, Kind, Thin,
+                      sys::path::filename(M.Buf->getBufferIdentifier()),
+                      StringMapIndexIter, M.ModTime, M.UID, M.GID, M.Perms,
+                      M.Buf->getBufferSize());
 
     if (!Thin)
       Out << File.getBuffer();
 
     if (Out.tell() % 2)
       Out << '\n';
   }
 

llvm/trunk/tools/llvm-ar/llvm-ar.cpp

if (!EC)
     return;
 
   std::string ContextStr = Context.str();
   if (ContextStr == "")
     fail(EC.message());
   fail(Context + ": " + EC.message());
 }
 
+static void failIfError(Error E, Twine Context = "") {
+  if (!E)
+    return;
+
+  handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EIB) {
+    std::string ContextStr = Context.str();
+    if (ContextStr == "")
+      fail(EIB.message());
+    fail(Context + ": " + EIB.message());
+  });
+}
+
 // llvm-ar/llvm-ranlib remaining positional arguments.
 static cl::list<std::string>
     RestOfArgs(cl::Positional, cl::ZeroOrMore,
                cl::desc("[relpos] [count] <archive-file> [members]..."));
 
 static cl::opt<bool> MRI("M", cl::desc(""));
 static cl::opt<std::string> Plugin("plugin", cl::desc("plugin (ignored for compatibility"));
 
static void performReadOperation(ArchiveOperation Operation,
   }
   if (Members.empty())
     return;
   for (StringRef Name : Members)
     errs() << Name << " was not found\n";
   std::exit(1);
 }
 
-static void addMember(std::vector<NewArchiveIterator> &Members,
+static void addMember(std::vector<NewArchiveMember> &Members,
                       StringRef FileName, int Pos = -1) {
-  NewArchiveIterator NI(FileName);
+  Expected<NewArchiveMember> NMOrErr =
+      NewArchiveMember::getFile(FileName, Deterministic);
+  failIfError(NMOrErr.takeError(), FileName);
   if (Pos == -1)
-    Members.push_back(NI);
+    Members.push_back(std::move(*NMOrErr));
   else
-    Members[Pos] = NI;
+    Members[Pos] = std::move(*NMOrErr);
 }
 
-static void addMember(std::vector<NewArchiveIterator> &Members,
-                      const object::Archive::Child &M, StringRef Name,
-                      int Pos = -1) {
+static void addMember(std::vector<NewArchiveMember> &Members,
+                      const object::Archive::Child &M, int Pos = -1) {
   if (Thin && !M.getParent()->isThin())
     fail("Cannot convert a regular archive to a thin one");
-  NewArchiveIterator NI(M, Name);
+  Expected<NewArchiveMember> NMOrErr =
+      NewArchiveMember::getOldMember(M, Deterministic);
+  failIfError(NMOrErr.takeError());
   if (Pos == -1)
-    Members.push_back(NI);
+    Members.push_back(std::move(*NMOrErr));
   else
-    Members[Pos] = NI;
+    Members[Pos] = std::move(*NMOrErr);
 }
 
 enum InsertAction {
   IA_AddOldMember,
   IA_AddNewMeber,
   IA_Delete,
   IA_MoveOldMember,
   IA_MoveNewMember
if (PosName.empty())
       return IA_AddNewMeber;
     return IA_MoveNewMember;
   }
   llvm_unreachable("No such operation");
 }
 
 // We have to walk this twice and computing it is not trivial, so creating an
 // explicit std::vector is actually fairly efficient.
-static std::vector<NewArchiveIterator>
+static std::vector<NewArchiveMember>
 computeNewArchiveMembers(ArchiveOperation Operation,
                          object::Archive *OldArchive) {
-  std::vector<NewArchiveIterator> Ret;
-  std::vector<NewArchiveIterator> Moved;
+  std::vector<NewArchiveMember> Ret;
+  std::vector<NewArchiveMember> Moved;
   int InsertPos = -1;
   StringRef PosName = sys::path::filename(RelPos);
   if (OldArchive) {
     for (auto &ChildOrErr : OldArchive->children()) {
       failIfError(ChildOrErr.getError());
       auto &Child = ChildOrErr.get();
       int Pos = Ret.size();
       ErrorOr<StringRef> NameOrErr = Child.getName();
       failIfError(NameOrErr.getError());
       StringRef Name = NameOrErr.get();
       if (Name == PosName) {
         assert(AddAfter || AddBefore);
         if (AddBefore)
           InsertPos = Pos;
         else
           InsertPos = Pos + 1;
       }
 
       std::vector<StringRef>::iterator MemberI = Members.end();
       InsertAction Action =
           computeInsertAction(Operation, Child, Name, MemberI);
       switch (Action) {
       case IA_AddOldMember:
-        addMember(Ret, Child, Name);
+        addMember(Ret, Child);
         break;
       case IA_AddNewMeber:
         addMember(Ret, *MemberI);
         break;
       case IA_Delete:
         break;
       case IA_MoveOldMember:
-        addMember(Moved, Child, Name);
+        addMember(Moved, Child);
         break;
       case IA_MoveNewMember:
         addMember(Moved, *MemberI);
         break;
       }
       if (MemberI != Members.end())
         Members.erase(MemberI);
     }
   }
 
   if (Operation == Delete)
     return Ret;
 
   if (!RelPos.empty() && InsertPos == -1)
     fail("Insertion point not found");
 
   if (RelPos.empty())
     InsertPos = Ret.size();
 
   assert(unsigned(InsertPos) <= Ret.size());
-  Ret.insert(Ret.begin() + InsertPos, Moved.begin(), Moved.end());
-
-  Ret.insert(Ret.begin() + InsertPos, Members.size(), NewArchiveIterator(""));
   int Pos = InsertPos;
+  for (auto &M : Moved) {
+    Ret.insert(Ret.begin() + Pos, std::move(M));
+    ++Pos;
+  }
+
+  for (unsigned I = 0; I != Members.size(); ++I)
+    Ret.insert(Ret.begin() + InsertPos, NewArchiveMember());
+  Pos = InsertPos;
   for (auto &Member : Members) {
     addMember(Ret, Member, Pos);
     ++Pos;
   }
 
   return Ret;
 }
 
 static object::Archive::Kind getDefaultForHost() {
   return Triple(sys::getProcessTriple()).isOSDarwin() ? object::Archive::K_BSD
                                                       : object::Archive::K_GNU;
 }
 
-static object::Archive::Kind
-getKindFromMember(const NewArchiveIterator &Member) {
-  auto getKindFromMemberInner =
-      [](MemoryBufferRef Buffer) -> object::Archive::Kind {
-    Expected<std::unique_ptr<object::ObjectFile>> OptionalObject =
-        object::ObjectFile::createObjectFile(Buffer);
+static object::Archive::Kind getKindFromMember(const NewArchiveMember &Member) {
+      object::ObjectFile::createObjectFile(Member.Buf->getMemBufferRef());
 
-    if (OptionalObject)
-      return isa<object::MachOObjectFile>(**OptionalObject)
-                 ? object::Archive::K_BSD
-                 : object::Archive::K_GNU;
 
-    // squelch the error in case we had a non-object file
-    consumeError(OptionalObject.takeError());
-    return getDefaultForHost();
-  };
-
-  if (Member.isNewMember()) {
-    object::Archive::Kind Kind = getDefaultForHost();
-
-    sys::fs::file_status Status;
-    if (auto OptionalFD = Member.getFD(Status)) {
-      if (auto MB = MemoryBuffer::getOpenFile(*OptionalFD, Member.getName(),
-                                              Status.getSize(), false))
-        Kind = getKindFromMemberInner((*MB)->getMemBufferRef());
-
-      if (close(*OptionalFD) != 0)
-        failIfError(std::error_code(errno, std::generic_category()),
-                    "failed to close file");
-    }
-
-    return Kind;
-  } else {
-    const object::Archive::Child &OldMember = Member.getOld();
-    if (OldMember.getParent()->isThin())
-      return object::Archive::Kind::K_GNU;
-
-    auto OptionalMB = OldMember.getMemoryBufferRef();
-    failIfError(OptionalMB.getError());
-
-    return getKindFromMemberInner(*OptionalMB);
-  }
 }
 
 static void
 performWriteOperation(ArchiveOperation Operation,
                       object::Archive *OldArchive,
                       std::unique_ptr<MemoryBuffer> OldArchiveBuf,
-                      std::vector<NewArchiveIterator> *NewMembersP) {
-  std::vector<NewArchiveIterator> NewMembers;
+                      std::vector<NewArchiveMember> *NewMembersP) {
+  std::vector<NewArchiveMember> NewMembers;
   if (!NewMembersP)
     NewMembers = computeNewArchiveMembers(Operation, OldArchive);
 
   object::Archive::Kind Kind;
   switch (FormatOpt) {
   case Default:
     if (Thin)
       Kind = object::Archive::K_GNU;
if (OldArchive->hasSymbolTable())
     return;
 
   performWriteOperation(CreateSymTab, OldArchive, nullptr, nullptr);
 }
 
 static void performOperation(ArchiveOperation Operation,
                              object::Archive *OldArchive,
                              std::unique_ptr<MemoryBuffer> OldArchiveBuf,
-                             std::vector<NewArchiveIterator> *NewMembers) {
+                             std::vector<NewArchiveMember> *NewMembers) {
   switch (Operation) {
   case Print:
   case DisplayTable:
   case Extract:
     performReadOperation(Operation, OldArchive);
     return;
 
   case Delete:
   case Move:
   case QuickAppend:
   case ReplaceOrInsert:
     performWriteOperation(Operation, OldArchive, std::move(OldArchiveBuf),
                           NewMembers);
     return;
   case CreateSymTab:
     createSymbolTable(OldArchive);
     return;
   }
   llvm_unreachable("Unknown operation.");
 }
 
 static int performOperation(ArchiveOperation Operation,
-                            std::vector<NewArchiveIterator> *NewMembers) {
+                            std::vector<NewArchiveMember> *NewMembers) {
   // Create or open the archive object.
   ErrorOr<std::unique_ptr<MemoryBuffer>> Buf =
       MemoryBuffer::getFile(ArchiveName, -1, false);
   std::error_code EC = Buf.getError();
   if (EC && EC != errc::no_such_file_or_directory)
     fail("error opening '" + ArchiveName + "': " + EC.message() + "!");
 
   if (!EC) {

 
 static void runMRIScript() {
   enum class MRICommand { AddLib, AddMod, Create, Save, End, Invalid };
 
   ErrorOr<std::unique_ptr<MemoryBuffer>> Buf = MemoryBuffer::getSTDIN();
   failIfError(Buf.getError());
   const MemoryBuffer &Ref = *Buf.get();
   bool Saved = false;
-  std::vector<NewArchiveIterator> NewMembers;
+  std::vector<NewArchiveMember> NewMembers;
   std::vector<std::unique_ptr<MemoryBuffer>> ArchiveBuffers;
   std::vector<std::unique_ptr<object::Archive>> Archives;
 
   for (line_iterator I(Ref, /*SkipBlanks*/ true, ';'), E; I != E; ++I) {
     StringRef Line = *I;
     StringRef CommandStr, Rest;
     std::tie(CommandStr, Rest) = Line.split(' ');
     Rest = Rest.trim();
case MRICommand::AddLib: {
       auto LibOrErr =
           object::Archive::create(ArchiveBuffers.back()->getMemBufferRef());
       failIfError(errorToErrorCode(LibOrErr.takeError()),
                   "Could not parse library");
       Archives.push_back(std::move(*LibOrErr));
       object::Archive &Lib = *Archives.back();
       for (auto &MemberOrErr : Lib.children()) {
         failIfError(MemberOrErr.getError());
-        auto &Member = MemberOrErr.get();
-        ErrorOr<StringRef> NameOrErr = Member.getName();
-        failIfError(NameOrErr.getError());
-        addMember(NewMembers, Member, *NameOrErr);
+        addMember(NewMembers, *MemberOrErr);
       }
       break;
     }
     case MRICommand::AddMod:
       addMember(NewMembers, Rest);
       break;
     case MRICommand::Create:
       Create = true;