[PATCH] D27874: ilist_iterator: Allow conversion between reverse and forward iterators
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 30 10:01:08 PST 2017
(+Kyle, since he noticed this first.)
E.g., getReverseWithoutShiftingBy1?
> On 2017-Jan-30, at 10:00, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
> I thing we should have *something* that does the natural thing, and I'm extremely hesitant to change the semantics of getReverse() since that was extremely error prone last time. Here's another possibility:
> - Rename getReverse() to some-great-new-name-that-is-clear.
> - Add conversions between forward/reverse do the same thing as std::reverse_iterator, with the off-by-1 thing.
>
> Any suggestions for something that's clear?
>
>> On 2017-Jan-30, at 09:49, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> 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
>>
>
More information about the llvm-commits
mailing list