[libcxx] r217902 - Create a 'comma_iterator' class that overloads operator, and asserts when it's called. Add tests to mismatch to make sure it can't be blindsided by such an evil iterator. More tests for other algorithms forthcoming. Thanks to STL for pointing this out at CppCon and Yakov Galka for opening LWG issue #2133

[Eric Fiselier]

I agree with David. By making it a compilation failure you get

a) Better error messages and it's easier to track down the location of the
failure (I spent a couple hours last night doing it w/ asserts and it was
not fun)
b) You will detect the call to operator comma even if that code is not
actually run.

Ensuring we never call operator comma should be sufficient to ensure that
the standard algorithms work with evil iterators.
Instead of using '= delete' I would just leave the method undefined to make
it c++03 compatible.

/Eric

On Thu, Oct 23, 2014 at 10:03 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Wed, Oct 22, 2014 at 6:05 PM, Marshall Clow <mclow.lists at gmail.com>
> wrote:
>
>>
>> On Sep 16, 2014, at 2:13 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Tue, Sep 16, 2014 at 1:38 PM, Marshall Clow <mclow.lists at gmail.com>
>> wrote:
>>
>>> Author: marshall
>>> Date: Tue Sep 16 15:38:11 2014
>>> New Revision: 217902
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=217902&view=rev
>>> Log:
>>> Create a 'comma_iterator' class that overloads operator, and asserts
>>> when it's called. Add tests to mismatch to make sure it can't be blindsided
>>> by such an evil iterator. More tests for other algorithms forthcoming.
>>> Thanks to STL for pointing this out at CppCon and Yakov Galka for opening
>>> LWG issue #2133
>>>
>>
>> Could it use "= delete" to make this test fail at compile-time?
>>
>>
>> Sorry I didn’t respond before.
>>
>> The point here is not that this should fail to compile, but rather that
>> such evil iterators should work with the standard algorithms.
>>
>
>> See  http://llvm.org/viewvc/llvm-project?rev=217903&view=rev for a
>> change to std::mismatch to work correctly.
>>
>
> Right - but what I mean is if the definition was "= deleted" you'd get
> compilation errors from the tests for std::mismatch before the (void) casts
> were added, right? I figured it just might be a lower-cost way to test (not
> needing to execute code) and give possibly easier to track down errors to
> fix algorithms that weren't comma overload safe.
>
> - David
>
>
>
>>
>> — Marshall
>>
>>
>>
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141023/dd384c35/attachment.html>