[PATCH] D22079: Refactor Archive-child iteration.

[Lang Hames via llvm-commits]

Hi Rafael,

Yes, sorry, it should be Expected<Optional<Child>>, which is verbose.


Expected<Optional<Child>> works for me. I'll update the patch.

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.


I think your change from Error* to Error& in the non-default constructor
makes this reasonably neat: default constructed iterators are "end"
markers, can't be incremented, and so don't need to have their Error* set.
On the other hand, child_iterator values constructed with a valid Child
will also have to have a valid Error reference passed in, and these are the
only iterators that can be incremented.

- Lang.

On Fri, Jul 8, 2016 at 12:56 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160708/30a00c7b/attachment.html>