Add 'operator==' for 'basic_collection_iterator'

[Alexey Denisov via llvm-commits]

> "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
>>>> 
>>> 
>> 
> 

-------------- 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/20151217/a3fd6f4d/attachment.sig>