<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 30, 2017, at 10:09 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Mon, Jan 30, 2017 at 10:00 AM Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I thing we should have *something* that does the natural thing,</blockquote><div class=""><br class=""></div><div class="">By 'natural thing' you mean the standard-esque off-by-one thing?</div></div></div></div></blockquote><div>At least to me the intuitive/natural behavior was to stay on the same object (current getReverse()). Probably similar to other people who haven't been using conversion from/to reverse std::reverse_iterator before. Though thinking about it/discussing this more, the STL like behavior does make sense: You want a conversion that works on any valid iterator and that includes end()/rend().</div><div><br class=""></div><div>IMO:</div><div>- If we add constructors we should implement the off-by-one behavior.</div><div>- I would have the tendency to not add any constructors to avoid problems with mismatched expectations, then again if someone has a good use for the functionality we should add it.</div><div>- I would not change the getReverse() behavior (why would you have a 2nd function that does exactly the same as the constructor?).</div><div>- We could however be more aggressive and remove getReverse(). At least in my code I ended up with the following pattern which works the same (for ilist_nodes), so maybe getReverse() is unnecessary:</div><div><br class=""></div><div> ilist_iterator I = ...;</div><div> auto R = I->getReverseIterator(); // dereference forward iterator and get reverse iterator to same object</div><div><br class=""></div><div>- Matthias</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> and I'm extremely hesitant to change the semantics of getReverse() since that was extremely error prone last time. Here's another possibility:<br class="gmail_msg">
- Rename getReverse() to some-great-new-name-that-is-clear.<br class="gmail_msg">
- Add conversions between forward/reverse do the same thing as std::reverse_iterator, with the off-by-1 thing.<br class="gmail_msg">
<br class="gmail_msg">
Any suggestions for something that's clear?<br class="gmail_msg">
<br class="gmail_msg">
> On 2017-Jan-30, at 09:49, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
> Maybe - I'm not sure the explicit ctor is sufficiently explicit from a self-documenting code perspective:<br class="gmail_msg">
><br class="gmail_msg">
> reverse_iterator R(I);<br class="gmail_msg">
> reverse_iterator R = I.getReverse();<br class="gmail_msg">
><br class="gmail_msg">
> I would be a bit surprised that those two things had different semantics.<br class="gmail_msg">
><br class="gmail_msg">
> On Mon, Jan 30, 2017 at 9:47 AM Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="gmail_msg" target="_blank">dexonsmith@apple.com</a>> wrote:<br class="gmail_msg">
> I think adding:<br class="gmail_msg">
> --<br class="gmail_msg">
> explicit iterator(reverse_iterator I) : I(++I.getReverse().base()) {}<br class="gmail_msg">
> explicit reverse_iterator(iterator I) : I(++I.getReverse().base()) {}<br class="gmail_msg">
> --<br class="gmail_msg">
> should fix the problem.<br class="gmail_msg">
> - getReverse() keeps the current semantics of getting the reverse of whatever you're currently referencing.<br class="gmail_msg">
> - conversions between forward/reverse do the same thing as std::reverse_iterator, with the off-by-1 thing.<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> > On 2017-Jan-30, at 09:43, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:<br class="gmail_msg">
> ><br class="gmail_msg">
> > 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.<br class="gmail_msg">
> ><br class="gmail_msg">
> > On Wed, Jan 25, 2017 at 3:33 PM Matthias Braun via Phabricator via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg">
> > MatzeB abandoned this revision.<br class="gmail_msg">
> > MatzeB added a comment.<br class="gmail_msg">
> ><br class="gmail_msg">
> > 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.<br class="gmail_msg">
> ><br class="gmail_msg">
> ><br class="gmail_msg">
> > Repository:<br class="gmail_msg">
> > rL LLVM<br class="gmail_msg">
> ><br class="gmail_msg">
> > <a href="https://reviews.llvm.org/D27874" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D27874</a><br class="gmail_msg">
> ><br class="gmail_msg">
> ><br class="gmail_msg">
> ><br class="gmail_msg">
> > _______________________________________________<br class="gmail_msg">
> > llvm-commits mailing list<br class="gmail_msg">
> > <a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
><br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>
</div></blockquote></div><br class=""></body></html>