Add 'operator==' for 'basic_collection_iterator'

[Jordan Rose via llvm-commits]

Your test sequences only have one element. If they had two elements, then "std::next(begin)" would have the same 'Base' as "begin", but a different 'CurrentEntry'.

Jordan

> On Dec 17, 2015, at 2:04 , Alexey Denisov <1101.debian at gmail.com> wrote:
> 
>> "std::next(begin) == begin" should not succeed either.
> 
> Indeed, it doesn’t: operator== returns false, since ’std::next(begin)' equals to '++begin’ and equals to ‘end’ in the particular test.
> 
>> the existing "end == end" test will probably fail as well.
> 
> As far as I see it behaves correctly, since I took ‘operator!=’s behaviour as a point of truth, i.e.: ‘end != end -> false’.
> 
> 
> Currently I’m confused a bit. Let me describe the flow to explain source of confusion:
> 
> When pointed me to the lack of tests I did the following:
> 
> 1. Rolled back my changes
> 2. Implemented all the tests presented in the most recent patch
> 3. Implemented ‘operator==‘ to return false -> some ‘==‘-related tests failed
> 4. Implemented ‘operator==‘ as negation of ‘operator!=‘ (i.e.: ‘!(*this != Other)') -> tests passed
> 5. Moved inverted implementation of ‘operator!=‘ into ‘operator==‘ -> tests passed
> 6. Implemented ‘operator!=‘ as negation of ‘operator==‘ (i.e.: ‘!(*this == Other)’) -> tests passed
> 
> So I still didn’t get what is wrong with the implementation.
> 
> I would appreciate if you can shed more light on the problem.
> --
> AlexDenisov
> Software Engineer, http://lowlevelbits.org
> 
>> On 17 Dec 2015, at 03:46, Jordan Rose <jordan_rose at apple.com> wrote:
>> 
>> Well, the YAML sequences aren't really re-entrant, but "std::next(begin) == begin" should not succeed either. And then once that's fixed, the existing "end == end" test will probably fail as well.
>> 
>> Besides that, though, the tests look good! Thanks for doing this.
>> Jordan
>> 
>>> On Dec 16, 2015, at 14:03 , Alexey Denisov <1101.debian at gmail.com> wrote:
>>> 
>>> Hmm, I did add couple of tests, but can’t catch wrong behaviour.
>>> What am I missing here?..
>>> 
>>> <basic_collection_iterator_operator_equals.patch>
>>> 
>>> --
>>> AlexDenisov
>>> Software Engineer, http://lowlevelbits.org
>>> 
>>>> On 15 Dec 2015, at 23:59, Jordan Rose <jordan_rose at apple.com> wrote:
>>>> 
>>>> The logic here is wrong:
>>>> 
>>>>> +  bool operator==(const basic_collection_iterator &Other) const {
>>>>> +    if (Base == Other.Base)
>>>>>     return true;
>>>> 
>>>> This test is backwards (early exit on !=, not ==), and the null check below is now incorrect.
>>>> 
>>>> I generally do support being consistent about operators, so I'm in favor of the patch, but please write some tests as well. unittests/Support/YAMLParserTest.cpp is probably a good place.
>>>> 
>>>> Jordan
>>>> 
>>>> 
>>>>> On Dec 15, 2015, at 13:14 , Alexey Denisov <1101.debian at gmail.com> wrote:
>>>>> 
>>>>> Patch adds ‘operator==‘ implementation for ‘basic_collection_iterator’
>>>>> 
>>>>> Motivation:
>>>>> 
>>>>> Swift compiler uses workaround since 'operator==‘ is not implemented.
>>>>> 
>>>>> Driver.cpp:220
>>>>> 
>>>>> if (!(seqI != seqE))
>>>>> return true;
>>>>> 
>>>>> 
>>>>> 
>>>>> <basic_collection_iterator_operator_equals.patch>
>>>>> 
>>>>> --
>>>>> AlexDenisov
>>>>> Software Engineer, http://lowlevelbits.org
>>>>> 
>>>> 
>>> 
>> 
>