[PATCH] D27874: ilist_iterator: Allow conversion between reverse and forward iterators

[David Blaikie via llvm-commits]

Maybe - I'm not sure the explicit ctor is sufficiently explicit from a
self-documenting code perspective:

reverse_iterator R(I);
reverse_iterator R = I.getReverse();

I would be a bit surprised that those two things had different semantics.

On Mon, Jan 30, 2017 at 9:47 AM Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> I think adding:
> --
> explicit iterator(reverse_iterator I) : I(++I.getReverse().base()) {}
> explicit reverse_iterator(iterator I) : I(++I.getReverse().base()) {}
> --
> should fix the problem.
> - getReverse() keeps the current semantics of getting the reverse of
> whatever you're currently referencing.
> - conversions between forward/reverse do the same thing as
> std::reverse_iterator, with the off-by-1 thing.
>
>
>
> > On 2017-Jan-30, at 09:43, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Yeah, I think it might be too subtle to have these reverse iterators
> behave differently from other reverse iterators. (I haven't checked the
> spec in detail to see if it's only stD::reverse_iterator that has this
> oddity, or if all reverse iterators must do so) 'getReverse' seems like a
> good/clear solution, if a little odd but at least visibly so.
> >
> > On Wed, Jan 25, 2017 at 3:33 PM Matthias Braun via Phabricator via
> llvm-commits <llvm-commits at lists.llvm.org> wrote:
> > MatzeB abandoned this revision.
> > MatzeB added a comment.
> >
> > I wasn't aware of the subtle shift-by-1 semantics when converting to
> std::reverse_iterator (must have missed the getReverse() comment). If we go
> with that semantics I would need extra std::next/std::prev calls in my code
> anyway which doesn't make it much simpler compared to
> MyIterator->getReverseIterator(). Going for semantics different from STL is
> not a good idea IMO.
> >
> >
> > Repository:
> >   rL LLVM
> >
> > https://reviews.llvm.org/D27874
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170130/15467077/attachment.html>