These are the matching changes needed to the lld project for the changes to llvm that changed the Archive and Child interfaces in libObject.

[enderby]

Summary

Lang Hames has given the “looks good to me” for these lld changes. But Rafael and Rui should OK these too.

[ruiu]

COFF/DriverUtils.cpp

525–528

error() does not return, so you can just do like this

error(ChildOrErr.getError(), "Archive::Child::getName failed");

COFF/InputFiles.cpp

71–74

Ditto

80–84

This looks odd. Why do you have to check for errors twice?

[enderby]

Addressed Rui's comments with the updated diff.

[ruiu]

LGTM with a few nits.

COFF/DriverUtils.cpp

525

error's first parameter can be ErrorOr<T>, so it can be

error(ChildOrErr, "Archive::Child::getName failed");

I'm sorry that I didn't mention this in the previous review.

COFF/InputFiles.cpp

71

Ditto

ELF/InputFiles.cpp

275

Remove break.

[enderby]

Updated the few nits that Rui pointed out. Also cleaned up further the code in
ArchiveFile::getMembers() on line 75 of ELF/InputFiles.cpp to use the same
error() like call to and replace the “if and break”. Thanks Rui!

[• rafael]

COFF/DriverUtils.cpp

524

can it be "const auto &"

ELF/InputFiles.cpp

255

You can write this as

ErrorOr<Child> &ChildOrErr = *It;
erorr(ChildOrErr, ...)

no?

[enderby]

Addressed Rafael's comments with the updated diff.

[enderby]

This final patch includes many suggested updates from Rafael Espindola emails. Rafael's last email contained:

On Nov 5, 2015, at 6:48 AM, Rafael Espíndola <[e-mail]> wrote:

LGTM with the "exit with 0 on error" issues fixed.

[enderby]

Closed with commit r252193

COFF/DriverUtils.cpp

ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr =
       MemoryBuffer::getFile(Path2, -1, false);
   error(BufOrErr, Twine("Failed to open ") + Path2);
   return MemoryBuffer::getMemBufferCopy((*BufOrErr)->getBuffer());
 }
 
 static std::vector<NewArchiveIterator>
 readMembers(const object::Archive &Archive) {
   std::vector<NewArchiveIterator> V;
-  for (const object::Archive::Child &C : Archive.children()) {
+  for (auto &ChildOrErr : Archive.children()) {
+    error(ChildOrErr, "Archive::Child::getName failed");
+    const object::Archive::Child C(*ChildOrErr);
     ErrorOr<StringRef> NameOrErr = C.getName();
     error(NameOrErr, "Archive::Child::getName failed");
     V.emplace_back(C, *NameOrErr);
   }
   return V;
 }
 
 // This class creates short import files which is described in
 // PE/COFF spec 7. Import Library Format.
 class ShortImportCreator {
NewArchiveIterator create(StringRef Sym, uint16_t Ordinal,
     Imp->TypeInfo = (isData ? IMPORT_DATA : IMPORT_CODE);
     Imp->TypeInfo |= NameType << 2;
 
     // Write symbol name and DLL name.
     memcpy(P, Sym.data(), Sym.size());
     P += Sym.size() + 1;
     memcpy(P, DLLName.data(), DLLName.size());
 
-    object::Archive::Child C(Parent, Buf);
+    object::Archive::Child C(Parent, Buf, nullptr);
     return NewArchiveIterator(C, DLLName);
   }
 
 private:
   BumpPtrAllocator Alloc;
   object::Archive *Parent;
   StringRef DLLName;
 };

COFF/InputFiles.cpp

void ArchiveFile::parse() {
 
   // Read the symbol table to construct Lazy objects.
   for (const Archive::Symbol &Sym : File->symbols())
     LazySymbols.emplace_back(this, Sym);
 
   // Seen is a map from member files to boolean values. Initially
   // all members are mapped to false, which indicates all these files
   // are not read yet.
-  for (const Archive::Child &Child : File->children())
+  for (auto &ChildOrErr : File->children()) {
+    error(ChildOrErr, "Failed to parse static library");
+    const Archive::Child &Child = *ChildOrErr;
     Seen[Child.getChildOffset()].clear();
   }
+}
 
 // Returns a buffer pointing to a member file containing a given symbol.
 // This function is thread-safe.
 MemoryBufferRef ArchiveFile::getMember(const Archive::Symbol *Sym) {
   auto ItOrErr = Sym->getMember();
   error(ItOrErr,
         Twine("Could not get the member for symbol ") + Sym->getName());
   Archive::child_iterator It = *ItOrErr;
 
   // Return an empty buffer if we have already returned the same buffer.
-  if (Seen[It->getChildOffset()].test_and_set())
+  if (Seen[(*It)->getChildOffset()].test_and_set())
     return MemoryBufferRef();
-  ErrorOr<MemoryBufferRef> Ret = It->getMemoryBufferRef();
+  ErrorOr<MemoryBufferRef> Ret = (*It)->getMemoryBufferRef();
   error(Ret, Twine("Could not get the buffer for the member defining symbol ") +
                  Sym->getName());
   return *Ret;
 }
 
 void ObjectFile::parse() {
   // Parse a memory buffer as a COFF file.
   auto BinOrErr = createBinary(MB);

ELF/InputFiles.cpp

for (const Archive::Symbol &Sym : File->symbols())
     LazySymbols.emplace_back(this, Sym);
 }
 
 // Returns a buffer pointing to a member file containing a given symbol.
 MemoryBufferRef ArchiveFile::getMember(const Archive::Symbol *Sym) {
   ErrorOr<Archive::child_iterator> ItOrErr = Sym->getMember();
   error(ItOrErr, "Could not get the member for symbol " + Sym->getName());
   Archive::child_iterator It = *ItOrErr;
+  if (std::error_code EC = It->getError())
+    error(EC, "Could not get the member for defining symbol " + Sym->getName());
 
-  if (!Seen.insert(It->getChildOffset()).second)
+  if (!Seen.insert((*It)->getChildOffset()).second)
     return MemoryBufferRef();
 
-  ErrorOr<MemoryBufferRef> Ret = It->getMemoryBufferRef();
+  ErrorOr<MemoryBufferRef> Ret = (*It)->getMemoryBufferRef();
   error(Ret, "Could not get the buffer for the member defining symbol " +
                  Sym->getName());
   return *Ret;
 }
 
 std::vector<MemoryBufferRef> ArchiveFile::getMembers() {
   File = openArchive(MB);
 
   std::vector<MemoryBufferRef> Result;
-  for (const Archive::Child &Child : File->children()) {
+  for (auto &ChildOrErr : File->children()) {
+    error(ChildOrErr, "Could not get the child of the archive " +
+                          File->getFileName());
+    const Archive::Child Child(*ChildOrErr);
     ErrorOr<MemoryBufferRef> MbOrErr = Child.getMemoryBufferRef();
     error(MbOrErr, "Could not get the buffer for a child of the archive " +
                        File->getFileName());
     Result.push_back(MbOrErr.get());
   }
   return Result;
 }
 
 template <class ELFT>

lib/ReaderWriter/FileArchive.cpp

public:
 
   /// \brief Check if any member of the archive contains an Atom with the
   /// specified name and return the File object for that member, or nullptr.
   File *find(StringRef name, bool dataSymbolOnly) override {
     auto member = _symbolMemberMap.find(name);
     if (member == _symbolMemberMap.end())
       return nullptr;
     Archive::child_iterator ci = member->second;
+    if (ci->getError())
+      return nullptr;
 
     // Don't return a member already returned
-    ErrorOr<StringRef> buf = ci->getBuffer();
+    ErrorOr<StringRef> buf = (*ci)->getBuffer();
     if (!buf)
       return nullptr;
     const char *memberStart = buf->data();
     if (_membersInstantiated.count(memberStart))
       return nullptr;
     if (dataSymbolOnly && !isDataSymbol(ci, name))
       return nullptr;
 
public:
   }
 
   // Instantiate a member file containing a given symbol name.
   void preload(TaskGroup &group, StringRef name) override {
     auto member = _symbolMemberMap.find(name);
     if (member == _symbolMemberMap.end())
       return;
     Archive::child_iterator ci = member->second;
+    if (ci->getError())
+      return;
 
     // Do nothing if a member is already instantiated.
-    ErrorOr<StringRef> buf = ci->getBuffer();
+    ErrorOr<StringRef> buf = (*ci)->getBuffer();
     if (!buf)
       return;
     const char *memberStart = buf->data();
     if (_membersInstantiated.count(memberStart))
       return;
 
     std::lock_guard<std::mutex> lock(_mutex);
     if (_preloaded.find(memberStart) != _preloaded.end())
if (ec)
       return ec;
     if ((ec = buildTableOfContents()))
       return ec;
     return std::error_code();
   }
 
 private:
   std::error_code
-  instantiateMember(Archive::child_iterator member,
+  instantiateMember(Archive::child_iterator cOrErr,
                     std::unique_ptr<File> &result) const {
-    ErrorOr<llvm::MemoryBufferRef> mbOrErr = member->getMemoryBufferRef();
+    if (std::error_code ec = cOrErr->getError())
+      return ec;
+    Archive::child_iterator member = cOrErr->get();
+    ErrorOr<llvm::MemoryBufferRef> mbOrErr = (*member)->getMemoryBufferRef();
     if (std::error_code ec = mbOrErr.getError())
       return ec;
     llvm::MemoryBufferRef mb = mbOrErr.get();
     std::string memberPath = (_archive->getFileName() + "("
                            + mb.getBufferIdentifier() + ")").str();
 
     if (_logLoading)
       llvm::errs() << memberPath << "\n";
instantiateMember(Archive::child_iterator cOrErr,
     // so that the bufffer is deallocated when all the members are destructed.
     result->setSharedMemoryBuffer(_mb);
     return std::error_code();
   }
 
   // Parses the given memory buffer as an object file, and returns true
   // code if the given symbol is a data symbol. If the symbol is not a data
   // symbol or does not exist, returns false.
-  bool isDataSymbol(Archive::child_iterator member, StringRef symbol) const {
-    ErrorOr<llvm::MemoryBufferRef> buf = member->getMemoryBufferRef();
+  bool isDataSymbol(Archive::child_iterator cOrErr, StringRef symbol) const {
+    if (cOrErr->getError())
+      return false;
+    Archive::child_iterator member = cOrErr->get();
+    ErrorOr<llvm::MemoryBufferRef> buf = (*member)->getMemoryBufferRef();
     if (buf.getError())
       return false;
     std::unique_ptr<MemoryBuffer> mb(MemoryBuffer::getMemBuffer(
         buf.get().getBuffer(), buf.get().getBufferIdentifier(), false));
 
     auto objOrErr(ObjectFile::createObjectFile(mb->getMemBufferRef()));
     if (objOrErr.getError())
       return false;
std::error_code buildTableOfContents() {
     for (const Archive::Symbol &sym : _archive->symbols()) {
       StringRef name = sym.getName();
       ErrorOr<Archive::child_iterator> memberOrErr = sym.getMember();
       if (std::error_code ec = memberOrErr.getError())
         return ec;
       Archive::child_iterator member = memberOrErr.get();
       DEBUG_WITH_TYPE(
           "FileArchive",
-          llvm::dbgs() << llvm::format("0x%08llX ", member->getBuffer()->data())
+          llvm::dbgs() << llvm::format("0x%08llX ",
+                          (*member)->getBuffer()->data())
                        << "'" << name << "'\n");
       _symbolMemberMap.insert(std::make_pair(name, member));
     }
     return std::error_code();
   }
 
   typedef std::unordered_map<StringRef, Archive::child_iterator> MemberMap;
   typedef std::set<const char *> InstantiatedSet;