Add 'operator==' for 'basic_collection_iterator'
Alex Denisov via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 4 22:03:36 PST 2016
Ping :)
--
AlexDenisov
Software Engineer, http://lowlevelbits.org
> On 20 Dec 2015, at 10:24, Alex Denisov <1101.debian at gmail.com> wrote:
>
> Here is updated version. Hopefully comment and assert are expressive enough.
> Also, expressed operator!= as negation of operator==, to not copy-paste assertion.
>
> <basic_collection_iterator_operator_equals.patch>
>
> --
> AlexDenisov
> Software Engineer, http://lowlevelbits.org
>
>> On 19 Dec 2015, at 01:33, Jordan Rose <jordan_rose at apple.com> wrote:
>>
>> Yep, that's what I meant, but it needs a comment to explain why it's safe.
>>
>> The assertion I wanted was
>>
>> if (Base && Base == Other.Base)
>> assert(Base->CurrentEntry == Other->CurrentEntry && "appropriate message here");
>>
>> That'll get trivially compiled out in Release builds, but in Debug builds it will catch any changes to the YAML parser in the future that might break the assumption.
>>
>> Jordan
>>
>>> On Dec 18, 2015, at 13:11, Alex Denisov <1101.debian at gmail.com> wrote:
>>>
>>> So I changed iterator tag to input_iterator and re-implemented equality operators.
>>>
>>> Now it looks trivial:
>>>
>>> bool operator==(const basic_collection_iterator &Other) const {
>>> return Base == Other.Base;
>>> }
>>>
>>> See the whole patch:
>>>
>>> <basic_collection_iterator_operator_equals.patch>
>>> --
>>> AlexDenisov
>>> Software Engineer, http://lowlevelbits.org
>>>
>>>> On 18 Dec 2015, at 21:05, Alex Denisov <1101.debian at gmail.com> wrote:
>>>>
>>>>> It really shouldn't be defining itself as a forward iterator. I think we should just make it an input iterator
>>>>
>>>> Makes sense. I also looked at specification (§24.2.3, §24.2.5): the iterator doesn’t conform some requirements described there (e.g. multi-pass guarantee).
>>>>
>>>>> and assert that the entries are the same if the bases are the same.
>>>>
>>>> How do you want express the assertion? Do you mean semantic assertion e.g.: 'X == Y iff X.Base == Y.Base’ or something different?
>>>> --
>>>> AlexDenisov
>>>> Software Engineer, http://lowlevelbits.org
>>>>
>>>>> On 18 Dec 2015, at 19:22, Jordan Rose <jordan_rose at apple.com> wrote:
>>>>>
>>>>>
>>>>>> On Dec 18, 2015, at 5:49 , Alexey Denisov <1101.debian at gmail.com> wrote:
>>>>>>
>>>>>>> Ha, I was still thinking something simpler.
>>>>>>
>>>>>> I came up with the same solution when I woke up next morning :-D
>>>>>>
>>>>>>> EXPECT_FALSE(Begin == std::next(Begin));
>>>>>>> EXPECT_FALSE(std::next(Begin) == Begin);
>>>>>>
>>>>>> I have faced an issue with such tests. std::next in the case of SequenceNode mutates value in-place, so that iterators are equal.
>>>>>> But it could be tested using some other BaseT, not a SequenceNode. I also realised it afterwards.
>>>>>>
>>>>>>> InputIterators don't actually guarantee that this is valid, so we don't have to implement it, but I think we should do it just so other people don't get bitten. There's not much reason not to.
>>>>>
>>>>> I looked at basic_collection_iterator again. It really shouldn't be defining itself as a forward iterator. I think we should just make it an input iterator and assert that the entries are the same if the bases are the same.
>>>>>
>>>>> Jordan
>>>>
>>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 496 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160105/de00f652/attachment.sig>
More information about the llvm-commits
mailing list