Add 'operator==' for 'basic_collection_iterator'
Jordan Rose via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 16 18:46:21 PST 2015
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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151216/8938ba0b/attachment.html>
More information about the llvm-commits
mailing list