[PATCH] D22079: Refactor Archive-child iteration.

[Lang Hames via llvm-commits]

lhames created this revision.
lhames added reviewers: rafael, enderby, grosbach.
lhames added a subscriber: llvm-commits.
lhames set the repository for this revision to rL LLVM.
lhames changed the visibility of this Differential Revision from "Public (No Login Required)" to "All Users".

This patch changes the way Archive::child_iterator works to enable interoperability with Error/Expected.

Background: We want to be able to iterate over the children of an archive, but iterator increment may fail for archive members if the archive header is damaged. We cannot return an error from the increment operation itself as this would break the usual C++ iterator idioms and prevent the use of range-based for loops. Instead, our current solution is for the Archive::child_iterator to return an ErrorOr<Child> on dereference, and to check this inside the loop:

for (auto &ChildOrErr : A->children()) {
  if (!ChildOrErr)
    return ChildOrErr.takeError();
}

This scheme works with ErrorOr, but does not work if we switch to Expected as Expected values are non-copyable and so an iterator containing an Expected would also be non-copyable. This would, again, break the usual C++ iterator idioms.

To enable Archive::child_iterator to work with Error/Expected this patch refactors Archive::child_iterator to hold a Child and an Error*, instead of an ErrorOr<Child>. If an increment fails the error is reported to the client via the Error pointer, and the iterator set to a value equivalent to calling Archive::child_end(). This will cause a loop over the children to break out on error, at which point the client can inspect the Error value. 

The resulting archive-walking idiom is:

Error Err;
for (auto &Child : A->children(Err)) {
  // Child can be safely accessed.
  // Loop will terminate early if increment encounters an error, and the Err variable will be set.
}
if (Err)
  return Err;

It's unfortunate that the error test at the end is left up to the client as it could be forgotten during initial coding (forcing this check during dereference was one of the reasons the old idiom was chosen). On the other hand, Error provides strong runtime assertions on checking, so any omission will be spotted the first time the code is run.

Repository:
  rL LLVM

http://reviews.llvm.org/D22079

Files:
  include/llvm/Object/Archive.h
  include/llvm/Support/Error.h
  lib/ExecutionEngine/MCJIT/MCJIT.cpp
  lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
  lib/Object/Archive.cpp
  lib/Support/Error.cpp
  tools/dsymutil/BinaryHolder.cpp
  tools/llvm-ar/llvm-ar.cpp
  tools/llvm-cxxdump/llvm-cxxdump.cpp
  tools/llvm-nm/llvm-nm.cpp
  tools/llvm-objdump/MachODump.cpp
  tools/llvm-objdump/llvm-objdump.cpp
  tools/llvm-readobj/llvm-readobj.cpp
  tools/llvm-size/llvm-size.cpp
  tools/sancov/sancov.cc

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22079.63006.patch
Type: text/x-patch
Size: 33340 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160707/291d01b4/attachment.bin>