On Thu, May 22, 2025 at 1:42 PM Jonathan Wakely <jwak...@redhat.com> wrote:
> This looks to have been wrong since r0-125454-gea89b2482f97aa which
> introduced the predefined_ops.h. Since that change, the binary predicate
> passed to std::__unique_copy is _Iter_comp_iter, which takes arguments
> of the iterator type, not the iterator's value type.
>
> This removes the checks from the __unique_copy overloads and moves them
> into the second overload of std::unique_copy, where we have the original
> binary predicate, not the adapted one from predefined_ops.h.
>
> The third __unique_copy overload currently checks that the predicate is
> callable with the input range value type and the output range value
> type. This change alters that, so that we only ever check that the
> predicate can be called with two arguments of the same type. That is
> intentional, because calling the predicate with different types is a bug
> that will be fixed in a later commit (see PR libstdc++/120386).
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/120384
> * include/bits/stl_algo.h (__unique_copy): Remove all
> _BinaryPredicateConcept concept checks.
> (unique_copy): Check _BinaryPredicateConcept in overload that
> takes a predicate.
> * testsuite/25_algorithms/unique_copy/120384.cc: New test.
> ---
>
> Tested x86_64-linux.
>
Took me a bit of time to understand why the check is having the same value
type
two times. But after reading the second commit, it makes sense.
LGTM, thanks.
>
> libstdc++-v3/include/bits/stl_algo.h | 17 +++--------------
> .../25_algorithms/unique_copy/120384.cc | 12 ++++++++++++
> 2 files changed, 15 insertions(+), 14 deletions(-)
> create mode 100644
> libstdc++-v3/testsuite/25_algorithms/unique_copy/120384.cc
>
> diff --git a/libstdc++-v3/include/bits/stl_algo.h
> b/libstdc++-v3/include/bits/stl_algo.h
> index 71ead103d2bf..f5361aeab7e2 100644
> --- a/libstdc++-v3/include/bits/stl_algo.h
> +++ b/libstdc++-v3/include/bits/stl_algo.h
> @@ -932,11 +932,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> _OutputIterator __result, _BinaryPredicate __binary_pred,
> forward_iterator_tag, output_iterator_tag)
> {
> - // concept requirements -- iterators already checked
> -
> __glibcxx_function_requires(_BinaryPredicateConcept<_BinaryPredicate,
> - typename iterator_traits<_ForwardIterator>::value_type,
> - typename iterator_traits<_ForwardIterator>::value_type>)
> -
> _ForwardIterator __next = __first;
> *__result = *__first;
> while (++__next != __last)
> @@ -962,11 +957,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> _OutputIterator __result, _BinaryPredicate __binary_pred,
> input_iterator_tag, output_iterator_tag)
> {
> - // concept requirements -- iterators already checked
> -
> __glibcxx_function_requires(_BinaryPredicateConcept<_BinaryPredicate,
> - typename iterator_traits<_InputIterator>::value_type,
> - typename iterator_traits<_InputIterator>::value_type>)
> -
> typename iterator_traits<_InputIterator>::value_type __value =
> *__first;
> __decltype(__gnu_cxx::__ops::__iter_comp_val(__binary_pred))
> __rebound_pred
> @@ -995,10 +985,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> _ForwardIterator __result, _BinaryPredicate
> __binary_pred,
> input_iterator_tag, forward_iterator_tag)
> {
> - // concept requirements -- iterators already checked
> -
> __glibcxx_function_requires(_BinaryPredicateConcept<_BinaryPredicate,
> - typename iterator_traits<_ForwardIterator>::value_type,
> - typename iterator_traits<_InputIterator>::value_type>)
> *__result = *__first;
> while (++__first != __last)
> if (!__binary_pred(__result, __first))
> @@ -4505,6 +4491,9 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
> __glibcxx_function_requires(_OutputIteratorConcept<_OutputIterator,
> typename iterator_traits<_InputIterator>::value_type>)
> __glibcxx_requires_valid_range(__first, __last);
> +
> __glibcxx_function_requires(_BinaryPredicateConcept<_BinaryPredicate,
> + typename iterator_traits<_InputIterator>::value_type,
> + typename iterator_traits<_InputIterator>::value_type>)
>
> if (__first == __last)
> return __result;
> diff --git a/libstdc++-v3/testsuite/25_algorithms/unique_copy/120384.cc
> b/libstdc++-v3/testsuite/25_algorithms/unique_copy/120384.cc
> new file mode 100644
> index 000000000000..27cd3375acae
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/25_algorithms/unique_copy/120384.cc
> @@ -0,0 +1,12 @@
> +// { dg-options "-D_GLIBCXX_CONCEPT_CHECKS" }
> +// { dg-do compile }
> +
> +// PR 120384 _BinaryPredicateConcept checks in std::unique_copy are wrong
> +
> +#include <algorithm>
> +
> +void
> +test_pr120384(const int* first, const int* last, int* out)
> +{
> + std::unique_copy(first, last, out);
> +}
> --
> 2.49.0
>
>
