This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Add irreflexive comparison debug check


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]