On 12 September 2016 at 13:41, Jonathan Wakely <jwak...@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.
