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

[Jonathan Wakely]

On 12/09/16 14:00 +0300, Ville Voutilainen wrote:
>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.

It's considerably lighter in terms of compilation cost. Defining a new
primary template adds more overhead than an alias template that uses
an already defined template.

>>> +  // 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.

OK, that makes sense.

>>> 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.

While you certainly know the pair constructors better than I do, I
don't think you can for a valid iterator type. Please don't waste your
time trying though :-)

>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

OK, if it bothers me enough I'll try to change the spec instead!

>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.

Right, there should be no conversions here. And iterators need to be
CopyConstructible. So the "as-if" rule applies. Using make_pair is
fine though.

As I said from my other email account, OK for trunk.