Re: [v3 PATCH] Implement C++17 string searchers.

[Ville Voutilainen <ville dot voutilainen at gmail dot com>]

On 12 September 2016 at 13:41, Jonathan Wakely <jwakely@redhat.com> wrote:
>> +  template<typename _Pred>
>> +    struct __is_std_equal_to : std::false_type { };
>> +
>> +  template<>
>> +    struct __is_std_equal_to<std::equal_to<void>> : std::true_type { };
>
>
> Is there a reason I didn't use an alias template or variable template here?
>
>  template<typename _Pred>
>    using __is_std_equal_to = is_same<equal_to<void>, _Pred>;
>
> That avoids defining a new class template.

I don't know whether that's a practical difference, the alias is
shorter of course.

>
>> +  // Use __boyer_moore_array_base when pattern consists of narrow
>> characters
>> +  // and uses std::equal_to as the predicate.
>> +  template<typename _RAIter, typename _Hash, typename _Pred,
>> +           typename _Val = typename iterator_traits<_RAIter>::value_type,
>> +          typename _Diff = typename
>> iterator_traits<_RAIter>::difference_type>
>> +    using __boyer_moore_base_t
>> +      = std::conditional_t<sizeof(_Val) == 1 && is_integral<_Val>::value
>> +                          && __is_std_equal_to<_Pred>::value,
>
>
> Could be __and_<is_integral<_Val>, __is_std_equal_to<_Pred>>::value
> but it doesn't make a lot of difference.

I didn't change any of those parts in the patch, I intentionally
avoided such changes.

>> std::get<1>(_M_m)));
>> +         return std::make_pair(__first_ret, __second_ret);
>
>
> This could be simply return { __first_ret, __second_ret };

That doesn't mean exactly the same thing. I can potentially concoct
evil code for which the result
is different with such a return and make_pair. I don't want to play
any games here, and I don't want the users to do so.
See below.

> Does using make_pair have any advantage? (I don't think we need to

No advantage as such, but for boyer_moore_searcher and
boyer_moore_horspool_searcher
the spec says make_pair, so I used make_pair everywhere. The spec says
nothing about
default_searcher, but I agreed with Marshall that we won't talk about
that and will just do the
same kind of initialization in all searchers.

> worry about iterators with explicit copy constructors.)

I would be more worried about iterators with explicit conversions, but
I don't think that will actually happen
because there shouldn't be a conversion involved, the incoming type
should be _ForwardIterator2 or _RandomAccessIterator2
and the outgoing type would be a pair either of those, so indeed there
should be at best a copy or move.