<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Well, the YAML sequences aren't <i class="">really</i> 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.</div><div class=""><br class=""></div><div class="">Besides that, though, the tests look good! Thanks for doing this.</div><div class="">Jordan</div><br class=""><div><blockquote type="cite" class=""><div class="">On Dec 16, 2015, at 14:03 , Alexey Denisov <<a href="mailto:1101.debian@gmail.com" class="">1101.debian@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hmm, I did add couple of tests, but can’t catch wrong behaviour.<br class="">What am I missing here?..<br class=""><br class=""><span id="cid:3175B2CD-B144-4840-9DCE-A871AEA82BBD"><basic_collection_iterator_operator_equals.patch></span><br class=""><br class="">--<br class="">AlexDenisov<br class="">Software Engineer, <a href="http://lowlevelbits.org" class="">http://lowlevelbits.org</a><br class=""><br class=""><blockquote type="cite" class="">On 15 Dec 2015, at 23:59, Jordan Rose <<a href="mailto:jordan_rose@apple.com" class="">jordan_rose@apple.com</a>> wrote:<br class=""><br class="">The logic here is wrong:<br class=""><br class=""><blockquote type="cite" class="">+ bool operator==(const basic_collection_iterator &Other) const {<br class="">+ if (Base == Other.Base)<br class=""> return true;<br class=""></blockquote><br class="">This test is backwards (early exit on !=, not ==), and the null check below is now incorrect.<br class=""><br class="">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.<br class=""><br class="">Jordan<br class=""><br class=""><br class=""><blockquote type="cite" class="">On Dec 15, 2015, at 13:14 , Alexey Denisov <<a href="mailto:1101.debian@gmail.com" class="">1101.debian@gmail.com</a>> wrote:<br class=""><br class="">Patch adds ‘operator==‘ implementation for ‘basic_collection_iterator’<br class=""><br class="">Motivation:<br class=""><br class="">Swift compiler uses workaround since 'operator==‘ is not implemented.<br class=""><br class="">Driver.cpp:220<br class=""><br class="">if (!(seqI != seqE))<br class=""> return true;<br class=""><br class=""><br class=""><br class=""><basic_collection_iterator_operator_equals.patch><br class=""><br class="">--<br class="">AlexDenisov<br class="">Software Engineer, <a href="http://lowlevelbits.org" class="">http://lowlevelbits.org</a><br class=""><br class=""></blockquote><br class=""></blockquote><br class=""></div></div></blockquote></div><br class=""></body></html>