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

79–83

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

315

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–526

can it be "const auto &"

ELF/InputFiles.cpp

295

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 (const 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 COrErr = Sym->getMember();
   error(COrErr, Twine("Could not get the member for symbol ") + Sym->getName());
   const Archive::Child &C = *COrErr;
 
   // Return an empty buffer if we have already returned the same buffer.
   if (Seen[C.getChildOffset()].test_and_set())
     return MemoryBufferRef();
   ErrorOr<MemoryBufferRef> Ret = C.getMemoryBufferRef();
   error(Ret, Twine("Could not get the buffer for the member defining symbol ") +
                  Sym->getName());
   return *Ret;
 }

ELF/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);
 }
 
 // Returns a buffer pointing to a member file containing a given symbol.
 MemoryBufferRef ArchiveFile::getMember(const Archive::Symbol *Sym) {
   ErrorOr<Archive::Child> COrErr = Sym->getMember();
   error(COrErr, "Could not get the member for symbol " + Sym->getName());
   const Archive::Child &C = *COrErr;
 
   if (!Seen.insert(C.getChildOffset()).second)
     return MemoryBufferRef();
 
   ErrorOr<MemoryBufferRef> Ret = C.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())
std::error_code doParse() override {
     if (ec)
       return ec;
     if ((ec = buildTableOfContents()))
       return ec;
     return std::error_code();
   }
 
 private:
-  std::error_code
-  instantiateMember(Archive::child_iterator member,
+  std::error_code 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";
std::error_code 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;
DEBUG_WITH_TYPE("FileArchive", llvm::dbgs()
                                        << "Table of contents for archive '"
                                        << _archive->getFileName() << "':\n");
     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())
+      DEBUG_WITH_TYPE("FileArchive",
+                      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;