[PATCH] D22079: Refactor Archive-child iteration.

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 7 11:34:26 PDT 2016


lhames added inline comments.

================
Comment at: include/llvm/Object/Archive.h:132
@@ -132,3 +131,3 @@
     child_iterator &operator++() { // Preincrement
-      assert(child && "Can't increment iterator with error");
-      child = child->getNext();
+      assert(E && "Can't increment iterator with no Error attached");
+      if (auto ChildOrErr = C.getNext())
----------------
rafael wrote:
> and this assert?
> 
> Basically this code has the invariant that E can be null, but we know it is not when needed, which is non-obvious.
> Basically this code has the invariant that E can be null, but we know it is not when needed, which is non-obvious.

Perhaps some extra documentation in the class header would help, but for clients nothing really changes here: child_begin now takes an Error& and will use this to report error if increment fails. The child_end() method doesn't take an Error&, and the returned iterator can't be incremented, but that was already true -- it would (hopefully) have segfaulted in Child::getNext when we try to access the null-initialized Data member.


Repository:
  rL LLVM

http://reviews.llvm.org/D22079





More information about the llvm-commits mailing list