[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