[PATCH] D55235: llvm-objcopy: Improve/simplify llvm::Error handling during notes iteration

[David Blaikie via Phabricator via llvm-commits]

dblaikie marked an inline comment as done.
dblaikie added inline comments.


================
Comment at: include/llvm/Object/ELFTypes.h:664
       : RemainingSize(Size), Err(&Err) {
+    consumeError(std::move(Err));
     assert(Start && "ELF note iterator starting at NULL");
----------------
MaskRay wrote:
> Why does here use `consumeError` but not `ErrorAsOutParameter`?
ErrorAsOutParameter essentially temporarily clears the "checked" state of the Error - inclearing the state at the end of the scope (when ErrorAsOutParameter's dtor is called).

Using ErrorAsOutParameter here would cause the caller to need to inspect the Error state even during an early return from within a loop - essentially when the iterator is incremented successfully, that's assumed to be "checked" (we could get fussier and only do that assumption when the increment was then compared to an end iterator and returned false - but in practice this is probably close enough).

I know this is a bit hairy & not super clear it's the best thing, but it's my best guess so far.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55235/new/

https://reviews.llvm.org/D55235