[PATCH] D57618: [ADT] Add a fallible_iterator wrapper.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht accepted this revision.
rupprecht marked an inline comment as done.
rupprecht added inline comments.
This revision is now accepted and ready to land.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:176
   }
   if (Err)
     return createFileError(Config.InputFilename, std::move(Err));
----------------
lhames wrote:
> rupprecht wrote:
> > When I remove this Error check at trunk, I can get 6 tests to fail:
> > 
> > ```
> > $ ninja check-llvm-tools-llvm-objcopy
> > ...
> > Program aborted due to an unhandled Error:
> > Error value was Success. (Note: Success values must still be checked prior to being destroyed).
> > ...
> > Failing Tests (6):
> >     LLVM :: tools/llvm-objcopy/ELF/basic-archive-copy.test
> >     LLVM :: tools/llvm-objcopy/ELF/deterministic-archive.test
> >     LLVM :: tools/llvm-objcopy/ELF/strip-all.test
> >     LLVM :: tools/llvm-objcopy/ELF/strip-debug.test
> >     LLVM :: tools/llvm-objcopy/ELF/strip-preserve-atime.test
> >     LLVM :: tools/llvm-objcopy/ELF/strip-preserve-mtime.test
> > ```
> > 
> > But when I patch in this change and then remove this line, I no longer get an error that Err is unchecked. So, I think something needs to reset Error so that it is unchecked at the end of normal loop iteration.
> I was not able to reproduce that. Can you try again with the latest patch? Is this happening on a debug or release branch?
I'm using a debug build w/ assertions enabled.

I tested with the latest version of the patch, and I'm getting the expected test failures now when I remove this error check, so LGTM. Thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57618