[PATCH] D22079: Refactor Archive-child iteration.

[Rafael Espíndola via llvm-commits]

On 8 July 2016 at 15:46, Lang Hames <lhames at gmail.com> wrote:
> lhames added a comment.
>
>> That would leave clients unable to detect the difference between a broken archive and an undefined symbol. I don't think we want to do that.
>
>
> More accurately - dropping the Error argument would leave us unable to tell the difference, but if I understand correctly dropping the error was the main motivation for switching to Optional.


Yes, sorry, it should be Expected<Optional<Child>>, which is verbose.
The reason I still like it best is that with the current interface it
is a bit confusing where exactly one should look for the error since
the use looks like

StructWithErrorInIt foo = func(Error &...)

It is not immediately obvious that one only needs to check error
passed to func. Also, while not nearly as bad as with error_code,
passing down an Error& is not as nice as returning an Expected except
when there is some other big gain (like being able to use a range
loop).

On the iterator not being default constructable: is that an actual
problem in here? The private constructor would make it easy to see
where the Error* can be null.

Cheers,
Rafael