Re: New antisymmetrical debug check

[FranÃois Dumont <frs dot dumont at gmail dot com>]

On 25/10/2015 18:55, FranÃois Dumont wrote:
> On 22/10/2015 21:41, Daniel KrÃgler wrote:
>> 2015-10-22 21:18 GMT+02:00 FranÃois Dumont <frs.dumont@gmail.com>:
>>> Hi
>>>
>>>     I leveraged on predefined operators to inject a new debug check for
>>> strict weak ordering operators. It checks that for a given functor f if
>>> f(a, b) then !f(b, a). In terms of < operator: if (a < b) assert(!(b < a)).
>>>
>>>     I had to replace the generic functors like _Iter_comp_iter by more
>>> explicit _Iter_less_iter where this check can take place. As algos can
>>> be used with different iterator types those checks are only performed in
>>> C++11 and after to make sure that b < a is a valid expression.
>> [..]
>>> Tested under Linux x86_64.
>>>
>>> Ok to commit ?
>> I'm wondering: Why does the _M_reverse_check overload set require
>> _GLIBCXX14_CONSTEXPR and not _GLIBCXX11_CONSTEXPR? None of them is
>> written in a way that it could not be a valid C++11 constexpr
>> function?
> Because it only needs _GLIBCXX14_CONSTEXPR, this is to make them usable
> for std::min, std::max, std::minmax, and some others.
>
>> I believe that operator()(_Iterator1 __it1, _Iterator2 __it2) could
>> easily be adapted to be also a valid C++11 constexpr function by
>> replacing the if-directive by an conditional expression and please
>> note that your __cplusplus tests are always C++11 compatible.
>>
>> On the other hand in line 311 of the patch you define a
>> _GLIBCXX14_CONSTEXPR operator(), but that one seems to invoke
>> _M_reverse_check functions where none of them is constexpr, so this
>> operator() could never be constexpr. Note also that
>> _Iter_less_val_comp on line 306 doesn't have a constexpr constructor,
>> so there is no way to invoke the constexpr operator() member.
>>
>> It seems as if a similar problem exists for line 377 and the following
>> _M_reverse_check overloads.
>>
>> Comparing to these two, why is in line 136 and line 187 the operator()
>> template and it's _M_reverse_check not constexpr as well?
> You are right, I haven't been careful enough with those. So I finally
> consider where those operators are used and kept only the mandatory
> ones. I kept _GLIBCXX14_CONSTEXPR also on the _M_reverse_check even if
> _GLIBCXX_DEBUG mode is not compatible with constexpr. You won't be able
> to build a constexpr when you activate it so those specification are not
> necessary.
>
> FranÃois
>
Hi

    I don't think I had the final decision regarding this patch ? Is
this new debug check ok ? If so is it too late ?

FranÃois