Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement P2443R1 views::chunk_by #2565

Merged
merged 8 commits into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 0 additions & 51 deletions stl/inc/algorithm
Original file line number Diff line number Diff line change
Expand Up @@ -406,57 +406,6 @@ _NODISCARD _FwdIt adjacent_find(_ExPo&& _Exec, const _FwdIt _First, const _FwdIt

#ifdef __cpp_lib_concepts
namespace ranges {
class _Adjacent_find_fn : private _Not_quite_object {
public:
using _Not_quite_object::_Not_quite_object;

template <forward_iterator _It, sentinel_for<_It> _Se, class _Pj = identity,
indirect_binary_predicate<projected<_It, _Pj>, projected<_It, _Pj>> _Pr = ranges::equal_to>
_NODISCARD constexpr _It operator()(_It _First, _Se _Last, _Pr _Pred = {}, _Pj _Proj = {}) const {
_Adl_verify_range(_First, _Last);

auto _UResult = _Adjacent_find_unchecked(
_Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Last)), _Pass_fn(_Pred), _Pass_fn(_Proj));

_Seek_wrapped(_First, _STD move(_UResult));
return _First;
}

template <forward_range _Rng, class _Pj = identity,
indirect_binary_predicate<projected<iterator_t<_Rng>, _Pj>, projected<iterator_t<_Rng>, _Pj>> _Pr =
ranges::equal_to>
_NODISCARD constexpr borrowed_iterator_t<_Rng> operator()(_Rng&& _Range, _Pr _Pred = {}, _Pj _Proj = {}) const {
auto _UResult = _Adjacent_find_unchecked(_Ubegin(_Range), _Uend(_Range), _Pass_fn(_Pred), _Pass_fn(_Proj));

return _Rewrap_iterator(_Range, _STD move(_UResult));
}

private:
template <class _It, class _Se, class _Pj, class _Pr>
_NODISCARD static constexpr _It _Adjacent_find_unchecked(_It _First, const _Se _Last, _Pr _Pred, _Pj _Proj) {
// find first satisfying _Pred with successor
_STL_INTERNAL_STATIC_ASSERT(forward_iterator<_It>);
_STL_INTERNAL_STATIC_ASSERT(sentinel_for<_Se, _It>);
_STL_INTERNAL_STATIC_ASSERT(indirect_binary_predicate<_Pr, projected<_It, _Pj>, projected<_It, _Pj>>);

if (_First == _Last) {
return _First;
}

for (auto _Next = _First;; ++_First) {
if (++_Next == _Last) {
return _Next;
}

if (_STD invoke(_Pred, _STD invoke(_Proj, *_First), _STD invoke(_Proj, *_Next))) {
return _First;
}
}
}
};

inline constexpr _Adjacent_find_fn adjacent_find{_Not_quite_object::_Construct_tag{}};

class _Count_fn : private _Not_quite_object {
public:
using _Not_quite_object::_Not_quite_object;
Expand Down
184 changes: 184 additions & 0 deletions stl/inc/ranges
Original file line number Diff line number Diff line change
Expand Up @@ -4782,6 +4782,190 @@ namespace ranges {
inline constexpr auto keys = elements<0>;
inline constexpr auto values = elements<1>;
} // namespace views

#if _HAS_CXX23
template <forward_range _Vw, indirect_binary_predicate<iterator_t<_Vw>, iterator_t<_Vw>> _Pr>
requires view<_Vw> && is_object_v<_Pr>
class chunk_by_view : public _Cached_position<_Vw, chunk_by_view<_Vw, _Pr>> {
private:
/* [[no_unique_address]] */ _Vw _Range{};
/* [[no_unique_address]] */ _Copyable_box<_Pr> _Pred{};
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

class _Iterator {
private:
friend chunk_by_view;

chunk_by_view* _Parent{};
/* [[no_unique_address]] */ iterator_t<_Vw> _Current{};
/* [[no_unique_address]] */ iterator_t<_Vw> _Next{};

constexpr _Iterator(chunk_by_view& _Parent_, iterator_t<_Vw> _Current_, iterator_t<_Vw> _Next_) noexcept(
is_nothrow_move_constructible_v<iterator_t<_Vw>>) // strengthened
: _Parent(_STD addressof(_Parent_)), _Current(_STD move(_Current_)), _Next(_STD move(_Next_)) {}

public:
using value_type = subrange<iterator_t<_Vw>>;
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
using difference_type = range_difference_t<_Vw>;
using iterator_category = input_iterator_tag;
using iterator_concept =
conditional_t<bidirectional_range<_Vw>, bidirectional_iterator_tag, forward_iterator_tag>;

_Iterator() = default;

_NODISCARD constexpr value_type operator*() const
noexcept(is_nothrow_copy_constructible_v<iterator_t<_Vw>>) /* strengthened */ {
#if _ITERATOR_DEBUG_LEVEL != 0
_STL_VERIFY(_Current != _Next, "cannot dereference chunk_by_view end iterator");
#endif // _ITERATOR_DEBUG_LEVEL != 0
return subrange{_Current, _Next};
}

constexpr _Iterator& operator++() {
#if _ITERATOR_DEBUG_LEVEL != 0
_STL_VERIFY(_Current != _Next, "cannot increment chunk_by_view iterator past end");
#endif // _ITERATOR_DEBUG_LEVEL != 0
_Current = _Next;
_Next = _Parent->_Find_next(_Next);
return *this;
}

constexpr _Iterator operator++(int) {
auto _Tmp = *this;
++*this;
return _Tmp;
}

constexpr _Iterator& operator--() requires bidirectional_range<_Vw> {
#if _ITERATOR_DEBUG_LEVEL != 0
_STL_VERIFY(_Parent != nullptr, "cannot decrement value-initialized chunk_by_view iterator");
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_STL_VERIFY(
_Current != _RANGES begin(_Parent->_Range), "cannot decrement chunk_by_view iterator before begin");
#endif // _ITERATOR_DEBUG_LEVEL != 0
_Next = _Current;
_Current = _Parent->_Find_prev(_Current);
return *this;
}

constexpr _Iterator operator--(int) requires bidirectional_range<_Vw> {
auto _Tmp = *this;
--*this;
return _Tmp;
}

_NODISCARD friend constexpr bool operator==(const _Iterator& _Left, const _Iterator& _Right) noexcept(
noexcept(_Implicitly_convert_to<bool>(_Left._Current == _Right._Current))) /* strengthened */ {
return _Left._Current == _Right._Current;
}

_NODISCARD friend constexpr bool operator==(const _Iterator& _Left, default_sentinel_t) noexcept(
noexcept(_Implicitly_convert_to<bool>(_Left._Current == _Left._Next))) /* strengthened */ {
return _Left._Current == _Left._Next;
}
};

_NODISCARD constexpr iterator_t<_Vw> _Find_next(iterator_t<_Vw> _It) {
#if _ITERATOR_DEBUG_LEVEL != 0
_STL_VERIFY(_Pred, "cannot increment a chunk_by_view iterator whose parent view has no predicate");
#endif // _ITERATOR_DEBUG_LEVEL != 0

const auto _Not_pred = [&_Orig_pred = *_Pred]<class _Ty1, class _Ty2>(_Ty1&& _Left, _Ty2&& _Right) {
return !_STD invoke(_Orig_pred, _STD forward<_Ty1>(_Left), _STD forward<_Ty2>(_Right));
};
const auto _Before_next = _RANGES adjacent_find(_It, _RANGES end(_Range), _Not_pred);
return _RANGES next(_Before_next, 1, _RANGES end(_Range));
}

_NODISCARD constexpr iterator_t<_Vw> _Find_prev(iterator_t<_Vw> _It) {
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
_STL_INTERNAL_STATIC_ASSERT(bidirectional_range<_Vw>);
#if _ITERATOR_DEBUG_LEVEL != 0
_STL_VERIFY(_Pred, "cannot decrement a chunk_by_view iterator whose parent view has no predicate");
#endif // _ITERATOR_DEBUG_LEVEL != 0

reverse_view _Rv{subrange{_RANGES begin(_Range), _It}};
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
const auto _Rev_not_pred = [&_Orig_pred = *_Pred]<class _Ty1, class _Ty2>(_Ty1&& _Left, _Ty2&& _Right) {
return !_STD invoke(_Orig_pred, _STD forward<_Ty2>(_Right), _STD forward<_Ty1>(_Left));
};
const auto _After_prev = _RANGES adjacent_find(_Rv, _Rev_not_pred);
return _RANGES prev(_After_prev.base(), 1, _RANGES begin(_Range));
}

public:
// clang-format off
chunk_by_view() requires default_initializable<_Vw> && default_initializable<_Pr> = default;
// clang-format on

constexpr explicit chunk_by_view(_Vw _Range_, _Pr _Pred_) noexcept(
is_nothrow_move_constructible_v<_Vw>&& is_nothrow_move_constructible_v<_Pr>) // strengthened
: _Range(_STD move(_Range_)), _Pred{in_place, _STD move(_Pred_)} {}

_NODISCARD constexpr _Vw base() const& noexcept(
is_nothrow_copy_constructible_v<_Vw>) /* strengthened */ requires copy_constructible<_Vw> {
return _Range;
}
_NODISCARD constexpr _Vw base() && noexcept(is_nothrow_move_constructible_v<_Vw>) /* strengthened */ {
return _STD move(_Range);
}

_NODISCARD constexpr const _Pr& pred() const noexcept /* strengthened */ {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Pred, "chunk_by_view has no predicate");
#endif // _CONTAINER_DEBUG_LEVEL > 0
return *_Pred;
}

_NODISCARD constexpr _Iterator begin() {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Pred, "cannot call begin on a chunk_by_view with no predicate");
#endif // _CONTAINER_DEBUG_LEVEL > 0

const auto _First = _RANGES begin(_Range);
if (this->_Has_cache()) {
return _Iterator{*this, _First, this->_Get_cache(_Range)};
}

const auto _Result = _Find_next(_First);
this->_Set_cache(_Range, _Result);
return _Iterator{*this, _First, _Result};
}

_NODISCARD constexpr auto end() {
if constexpr (common_range<_Vw>) {
const auto _Last = _RANGES end(_Range);
return _Iterator{*this, _Last, _Last};
} else {
return default_sentinel;
}
}
};

template <class _Rng, class _Pr>
chunk_by_view(_Rng&&, _Pr) -> chunk_by_view<views::all_t<_Rng>, _Pr>;

namespace views {
struct _Chunk_by_fn {
// clang-format off
template <viewable_range _Rng, class _Pr>
_NODISCARD constexpr auto operator()(_Rng&& _Range, _Pr&& _Pred) const noexcept(
noexcept(chunk_by_view(_STD forward<_Rng>(_Range), _STD forward<_Pr>(_Pred)))) requires requires {
chunk_by_view(_STD forward<_Rng>(_Range), _STD forward<_Pr>(_Pred));
} {
// clang-format on
return chunk_by_view(_STD forward<_Rng>(_Range), _STD forward<_Pr>(_Pred));
Comment on lines +4949 to +4953
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why, but I have observed that other views will switch to saying static_cast<T&&>(t) in their requires, despite our usual currently preferred convention of using forward elsewhere. Not sure if this matters here. Example:

STL/stl/inc/ranges

Lines 3806 to 3815 in afe0800

struct _Lazy_split_fn {
// clang-format off
template <viewable_range _Rng, class _Pat>
_NODISCARD constexpr auto operator()(_Rng&& _Range, _Pat&& _Pattern) const noexcept(noexcept(
lazy_split_view(_STD forward<_Rng>(_Range), _STD forward<_Pat>(_Pattern)))) requires requires {
lazy_split_view(static_cast<_Rng&&>(_Range), static_cast<_Pat&&>(_Pattern));
} {
// clang-format on
return lazy_split_view(_STD forward<_Rng>(_Range), _STD forward<_Pat>(_Pattern));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And filter has mixed static_cast and _STD forward.

STL/stl/inc/ranges

Lines 1841 to 1844 in afe0800

template <viewable_range _Rng, class _Pr>
_NODISCARD constexpr auto operator()(_Rng&& _Range, _Pr&& _Pred) const noexcept(noexcept(
filter_view(_STD forward<_Rng>(_Range), _STD forward<_Pr>(_Pred)))) requires requires {
filter_view(static_cast<_Rng&&>(_Range), _STD forward<_Pr>(_Pred));

And transform even uses _STD move.

STL/stl/inc/ranges

Lines 2300 to 2303 in afe0800

template <viewable_range _Rng, class _Fn>
_NODISCARD constexpr auto operator()(_Rng&& _Range, _Fn _Fun) const noexcept(noexcept(
transform_view(_STD forward<_Rng>(_Range), _STD move(_Fun)))) requires requires {
transform_view(static_cast<_Rng&&>(_Range), _STD move(_Fun));

@CaseyCarter do you remember why this was done in this way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't replace all uses of forward and move with static_cast due to readability concerns. As a special exception to that rule, we do write static_cast instead of forward and move in concepts and constraints because compiler throughput concerns trump readability - at least until we could complete everything and get real numbers - and we want to avoid instantiating templates that may only ever be used in failing constraint checks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coda: I didn't complain here - despite that this would seemingly violate the general guideline - since there are no competing overloads of "operator() with arity 2" and no views_chunkable concept. I wouldn't mind if you decide to change this for consistency, but I won't demand it either.

}

// clang-format off
template <class _Pr>
requires constructible_from<decay_t<_Pr>, _Pr>
_NODISCARD constexpr auto operator()(_Pr&& _Pred) const
noexcept(is_nothrow_constructible_v<decay_t<_Pr>, _Pr>) {
// clang-format on
return _Range_closure<_Chunk_by_fn, decay_t<_Pr>>{_STD forward<_Pr>(_Pred)};
}
};

inline constexpr _Chunk_by_fn chunk_by;
} // namespace views
#endif // _HAS_CXX23
} // namespace ranges

namespace views = ranges::views;
Expand Down
51 changes: 51 additions & 0 deletions stl/inc/xutility
Original file line number Diff line number Diff line change
Expand Up @@ -5752,6 +5752,57 @@ namespace ranges {

inline constexpr _Find_if_not_fn find_if_not{_Not_quite_object::_Construct_tag{}};

class _Adjacent_find_fn : private _Not_quite_object {
public:
using _Not_quite_object::_Not_quite_object;

template <forward_iterator _It, sentinel_for<_It> _Se, class _Pj = identity,
indirect_binary_predicate<projected<_It, _Pj>, projected<_It, _Pj>> _Pr = ranges::equal_to>
_NODISCARD constexpr _It operator()(_It _First, _Se _Last, _Pr _Pred = {}, _Pj _Proj = {}) const {
_Adl_verify_range(_First, _Last);

auto _UResult = _Adjacent_find_unchecked(
_Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Last)), _Pass_fn(_Pred), _Pass_fn(_Proj));

_Seek_wrapped(_First, _STD move(_UResult));
return _First;
}

template <forward_range _Rng, class _Pj = identity,
indirect_binary_predicate<projected<iterator_t<_Rng>, _Pj>, projected<iterator_t<_Rng>, _Pj>> _Pr =
ranges::equal_to>
_NODISCARD constexpr borrowed_iterator_t<_Rng> operator()(_Rng&& _Range, _Pr _Pred = {}, _Pj _Proj = {}) const {
auto _UResult = _Adjacent_find_unchecked(_Ubegin(_Range), _Uend(_Range), _Pass_fn(_Pred), _Pass_fn(_Proj));

return _Rewrap_iterator(_Range, _STD move(_UResult));
}

private:
template <class _It, class _Se, class _Pj, class _Pr>
_NODISCARD static constexpr _It _Adjacent_find_unchecked(_It _First, const _Se _Last, _Pr _Pred, _Pj _Proj) {
// find first satisfying _Pred with successor
_STL_INTERNAL_STATIC_ASSERT(forward_iterator<_It>);
_STL_INTERNAL_STATIC_ASSERT(sentinel_for<_Se, _It>);
_STL_INTERNAL_STATIC_ASSERT(indirect_binary_predicate<_Pr, projected<_It, _Pj>, projected<_It, _Pj>>);

if (_First == _Last) {
return _First;
}

for (auto _Next = _First;; ++_First) {
if (++_Next == _Last) {
return _Next;
}

if (_STD invoke(_Pred, _STD invoke(_Proj, *_First), _STD invoke(_Proj, *_Next))) {
return _First;
}
}
}
};

inline constexpr _Adjacent_find_fn adjacent_find{_Not_quite_object::_Construct_tag{}};

// clang-format off
template <class _It1, class _It2, class _Se2, class _Pr, class _Pj1, class _Pj2>
concept _Equal_rev_pred_can_memcmp = is_same_v<_Pj1, identity> && is_same_v<_Pj2, identity>
Expand Down
2 changes: 2 additions & 0 deletions stl/inc/yvals_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@
// P2136R3 invoke_r()
// P2166R1 Prohibiting basic_string And basic_string_view Construction From nullptr
// P2186R2 Removing Garbage Collection Support
// P2443R1 views::chunk_by

// Parallel Algorithms Notes
// C++ allows an implementation to implement parallel algorithms as calls to the serial algorithms.
Expand Down Expand Up @@ -1370,6 +1371,7 @@

#ifdef __cpp_lib_concepts
#define __cpp_lib_out_ptr 202106L
#define __cpp_lib_ranges_chunk_by 202202L
#define __cpp_lib_ranges_starts_ends_with 202106L
#endif // __cpp_lib_concepts

Expand Down
2 changes: 2 additions & 0 deletions tests/std/test.lst
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,8 @@ tests\P2162R2_std_visit_for_derived_classes_from_variant
tests\P2231R1_complete_constexpr_optional_variant
tests\P2401R0_conditional_noexcept_for_exchange
tests\P2415R2_owning_view
tests\P2443R1_views_chunk_by
tests\P2443R1_views_chunk_by_death
tests\VSO_0000000_allocator_propagation
tests\VSO_0000000_any_calling_conventions
tests\VSO_0000000_c_math_functions
Expand Down
4 changes: 4 additions & 0 deletions tests/std/tests/P2443R1_views_chunk_by/env.lst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

RUNALL_INCLUDE ..\strict_concepts_latest_matrix.lst
Loading