On 30/07/2015 12:30, Jonathan Wakely wrote:
> On 29/07/15 22:08 +0200, François Dumont wrote:
>> Hi
>>
>> Here is a patch to add irreflexive debug check.
>
> Awesome!
>
> You can add PR libstdc++/60519 to the changelog.
Sure.
>
>
>> Standard algos signatures are such that there is no guaranty that
>> the operator < or predicate to compare the iterator value type will
>> exist. So I had to check if the call is valid using C++11 features
>> declval and decltype.
>
> Surely if someone calls an algorithm that requires a
> StrictWeakOrdering then we know that it's valid to compare the value
> types using the comparison implied by that particular call (either
> operator< or the supplied predicate), and we should only be adding
> this assertion to algorithms that require a StrictWeakOrdering.
>
> Am I missing something?
If you consider for instance lower_bound signature:
template<typename _ForwardIterator, typename _Tp>
inline _ForwardIterator
lower_bound(_ForwardIterator __first, _ForwardIterator __last,
const _Tp& __val)
This algo require expressions *__first < __val or __val < *__first
to be well defined. But what the check is doing is *__first < *__first
that might not exist as the algo won't need it. The range [__first,
__last[ might have been sorted using another functor that we don't know
at the time of the call to lower_bound.
There might be situations where the check will be useless but I
prefer to keep this debug check simple and so always go through the check.
>
> Assuming those validity checks are needed ...
>
>> + struct _Irreflexive_checker
>> + {
>> +#if __cplusplus >= 201103L
>> + template<typename _It>
>> + using __it_ref_t = typename std::iterator_traits<_It>::reference;
>
> I'd just call this __ref_t, because where you use it the "it" part is
> already implied by the template argument: __ref_t<_It>
>
> Or you could add:
>
> template<typename _It>
> static typename std::iterator_traits<_It>::reference
> __deref();
>
> and replace every std::declval<__it_ref_t<_It>>() with __deref<_It>()
> which is much shorter.
Nice, I will use that.
>
>> + template<typename _It,
>> + typename = decltype(
>> + std::declval<__it_ref_t<_It> >() < std::declval<__it_ref_t<_It>
>> >())>
>
> N.B. as this is only for C++11 and later there's no need for the
> spaces before the closing >
You read my mind on this one, thanks.
François
