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