Additional tests for requires clause of oneapi::dpl::range algorithms - #2812
Open
SergeyKopienko wants to merge 162 commits into
Open
SergeyKopienko wants to merge 162 commits into
SergeyKopienko wants to merge 162 commits into
Conversation
SergeyKopienko
requested review from
MikeDvorskiy,
akukanov,
danhoeflinger and
dmitriy-sobolev
August 31, 2026 13:23
SergeyKopienko
force-pushed
the
dev/skopienko/extra_range_tests_for_value_requirements_full
branch
2 times, most recently
from
September 4, 2026 10:46
1910912 to
3dd554b
Compare
The vectorized min_element/minmax_element bricks keep copies of the values in a user-defined reduction object and compare those copies, which puts requirements on the value type and on the comparator that not every argument of the algorithms satisfies. Add a trait stating those requirements, so that the callers can check them. The trait is expressed with the concepts it stands for: in C++20 std::semiregular, std::convertible_to and std::predicate are used directly, in C++17 each of them is approximated. The approximations are named after the concepts and kept as close to them as the type traits allow; where they deviate, the comment says how.
Check the trait and every requirement it is built from, in both directions: a type that satisfies the requirement and a type that does not. The C++17 building blocks of __semiregular are checked as well, since in C++20 they do not exist.
…bricks __simd_min_element and __simd_minmax_element pass a pointer to the comparator into the reduction object, and the comparator is a user-provided type that may overload the unary operator&.
…applicable __brick_min_element and __brick_minmax_element called __simd_min_element and __simd_minmax_element for every value type and comparator, so an argument that does not satisfy the requirements of the reduction object used there failed to compile instead of falling back to the serial implementation. Guard both calls with __is_value_storable_and_comparable_v and state the same requirement as a static_assert in the bricks themselves.
…llback Check the value types that decide which code path is taken: one that is default-constructible only through an explicit default constructor, which the vectorized path still has to accept, and three that it has to reject, each of them violating one of the requirements - default construction, copy assignment and copy construction. All four fail to compile in this test unless the code path is chosen by the value type.
…n` / `max` / `minmax` path (#2821)
…al::__key_t -> __projected_value_t
…/extra_range_tests_for_value_requirements_full
…al::__key_t -> __projected_value_t
…/extra_range_tests_for_value_requirements_full
SergeyKopienko
force-pushed
the
dev/skopienko/extra_range_tests_for_value_requirements_full
branch
from
September 18, 2026 08:52
d59161d to
63c13f7
Compare
oneapi::dpl::__internal::__ref_or_copy is an alias that resolves to a reference for the host policies and to a
by-value copy for the device ones, so the way an object of that type is initialized is what decides whether the
argument is bound or copied. ranges::replace_copy_if passed the new value as
__ref_or_copy<_ExecutionPolicy, const _T>{__new_value}, and list-initialization of a reference type generates a
prvalue of the referenced type and binds the reference to it, i.e. it requires _T to be copy-constructible.
std::ranges::replace_copy_if requires only std::indirectly_writable<std::ranges::iterator_t<_OutR>, const _T&>,
so a value type with a deleted copy constructor is legal input. gcc rejects the braced form in every
instantiation reached by std_ranges_algo_archetypes_write.pass - 16 errors, all of them pointing at the
__pattern_replace_copy_if call - while clang-based compilers bind the reference directly and accept the same
code, which is why the defect stayed invisible in local testing.
Round brackets bind the reference to the caller's argument with no temporary in between, and nothing below
copies the value either: __pattern_replace_copy_if takes it as const _T& and __replace_copy_functor stores that
same reference type. The device path is unaffected - there __ref_or_copy is a value type, and both spellings
copy.
The alias itself now carries a comment stating the rule, because the difference between the two spellings shows
up in the host case only: the device flavour copies with either of them, so the mistake is easy to make and
easy to miss.
__parallel_stable_sort_body merged the two sorted halves into a std::vector of __size elements, and that container default constructs every element of the merge buffer. Neither std::sort nor std::ranges::sort asks for this: a sortable value type is only guaranteed to be movable, so sorting a type without a default constructor failed to compile with the OpenMP backend while the very same call built with the serial and the TBB backends. The TBB backend merges through a raw buffer of uninitialized memory, so do the same here. The direction of the two passes is reversed as well. Instead of merging the source range into the buffer and then moving the result back, the halves are first move constructed into the raw buffer and the buffer is then merged back into the source range. The merge now writes into elements that are still alive and needs move assignment only, so a single construct flavoured helper, __parallel_uninitialized_move_range, is enough, and the value initialization pass of the vector is gone on top of that.
__pattern_destroy for a serial policy passed the range as a whole to std::ranges::destroy. libc++, checked up to version 18, implements that overload through a helper whose iterator and sentinel share a single template parameter, so it rejects every range whose sentinel type is not its iterator type. A sized random access range is not required to be common, which is why std_ranges_memory_archetypes.pass failed to build in the libc++ configurations. Compute the end iterator from the size of the range and call the iterator and sentinel overload instead, exactly as the parallel overload right above already does.
…terns
The C++20 range patterns spelled the iterator pair of a sized random access
range out by hand, repeating
auto __first = std::ranges::begin(__r);
auto __last = __first + std::ranges::size(__r);
in every one of them. __ranges::__bounds() returns exactly that pair and is
already used by the other range patterns, so let the remaining ones use it too.
No functional change: the helper evaluates the same two expressions, and both
elements of the tuple it returns keep the iterator type of the range, which is
what the iterator based patterns below need - a sized random access range is
not required to be a common range.
SergeyKopienko
force-pushed
the
dev/skopienko/extra_range_tests_for_value_requirements_full
branch
2 times, most recently
from
September 18, 2026 09:35
4e362d3 to
ae9fbb6
Compare
…quirements_full # Conflicts: # include/oneapi/dpl/pstl/utils_ranges.h
__relax_const_pred relaxes the constness of the arguments of a predicate of any arity, so it already covers everything __relax_const_comp did: for a binary comparator its variadic operator() is the comparator call, and the return type it deduces is the bool such a comparator returns anyway. Two class templates for one behaviour also meant two detection traits, two forward declarations and two sycl::is_device_copyable specializations to keep in sync. Remove __relax_const_comp together with __comp_wants_mutable_args_v and turn __get_relax_non_const_comp into a thin wrapper over __get_relax_non_const_pred. The name is worth keeping rather than replacing at the call sites: a parameter pack cannot have a default argument, so the homogeneous comparison comp(_T&, _T&) can only be spelled with one type argument through a non-variadic signature, and that is the form 50 of the 74 call sites use. The wrapper is defined after __get_relax_non_const_pred because the name of the callee is looked up in the definition context. The detection of a comparator that wants mutable arguments now runs through is_invocable_v instead of is_invocable_r_v<bool, ...>. The two verdicts differ only for a callable that is invocable with const arguments while its result is not convertible to bool, which cannot happen for a comparator satisfying strict_weak_order, so no call site changes its behaviour.
SergeyKopienko
force-pushed
the
dev/skopienko/extra_range_tests_for_value_requirements_full
branch
from
September 18, 2026 10:05
a098620 to
895ff9e
Compare
… indirectly_storable.pass.cpp
…pienko/extra_range_tests_for_value_requirements_full
SergeyKopienko
force-pushed
the
dev/skopienko/extra_range_tests_for_value_requirements_full
branch
from
September 18, 2026 11:42
895ff9e to
1d0fd5e
Compare
…go2_all_policies for already resolved broken test cases
…_transform_fn::operator() + remove outdated comment from lexicographical_compare() call in std_ranges_algo_archetypes_cross.pass.cpp
… - remove extra comment
…__internal::__get_relax_non_const_comp() in the code - remove dummy comment
SergeyKopienko
force-pushed
the
dev/skopienko/extra_range_tests_for_value_requirements_full
branch
from
September 18, 2026 16:21
e6dd6d8 to
3f009ad
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.