| Message ID | 20200218191404.1118768-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libstdc++: P1983R0 Wording for GB301, US296, US292, US291, and US283 | expand |
On Tue, 18 Feb 2020, Patrick Palka wrote: > Among other changes, P1983R0 resolves LWG 3278 in a different way, so this patch > also reverts the already-applied change of LWG 3278. > > The wording for US283 has already been applied it seems. > > libstdc++-v3/ChangeLog: > > P1983R0 Wording for GB301, US296, US292, US291, and US283 > * include/std/ranges (filter_view::pred): New member function. > (join_view::_Iterator::_Iterator): Remove now-redundant comment since > P1983R0 fixes the highlighted issue in the same way. > (join_view::_M_inner): Remove mutable specifier, effectively reverting > the proposed wording changes of P3278. > (join_view::begin): Refine the condition for when to return a const > iterator. > (split_view::_OuterIter::_OuterIter): Adjust constraints. > * testsuite/std/ranges/adaptors/filter.cc: Test that filter_view::pred > exists and works. Oops, this patch is incomplete, it breaks the join_view adaptor test due to a missing friend, sorry about that. Here's the complete and tested patch: -- >8 -- Subject: [PATCH] libstdc++: P1983R0 Wording for GB301, US296, US292, US291, and US283 Among other changes, P1983R0 resolves LWG 3278 in a different way, so this patch also reverts the already-applied change of LWG 3278. The wording for US291 (the join_view::begin hunk) also required adding the friend _Iterator<!_Const> to join::view::_Iterator. This friend is required for one of this iterator's converting constructors to be able to access the private members of an _Iterator of the opposite constness. The wording for US283 has already been applied it seems. libstdc++-v3/ChangeLog: P1983R0 Wording for GB301, US296, US292, US291, and US283 * include/std/ranges (filter_view::pred): New member function. (join_view::_Iterator::_Iterator): Remove now-redundant comment since P1983R0 fixes the highlighted issue in the same way. (join_view::_Iterator<_Const>): Add friend join_view::_Iterator<!_Const>. (join_view::_M_inner): Remove mutable specifier, effectively reverting the proposed wording changes of P3278. (join_view::begin): Refine the condition for when to return a const iterator. (split_view::_OuterIter::_OuterIter): Adjust constraints. * testsuite/std/ranges/adaptors/filter.cc: Test that filter_view::pred exists and works. --- libstdc++-v3/include/std/ranges | 18 +++++++++++------- .../testsuite/std/ranges/adaptors/filter.cc | 2 ++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges index a04387a75c4..fb6e67dd0b3 100644 --- a/libstdc++-v3/include/std/ranges +++ b/libstdc++-v3/include/std/ranges @@ -1556,6 +1556,10 @@ namespace views base() && { return std::move(_M_base); } + constexpr const _Pred& + pred() const + { return *_M_pred; } + constexpr _Iterator begin() { @@ -2421,8 +2425,6 @@ namespace views _Iterator() = default; - // XXX: had to change the type of __outer from iterator_t<_Vp> to - // iterator_t<_Base> here, a possible defect in the spec? constexpr _Iterator(_Parent& __parent, iterator_t<_Base> __outer) : _M_outer(std::move(__outer)), @@ -2523,6 +2525,7 @@ namespace views noexcept(noexcept(ranges::iter_swap(__x._M_inner, __y._M_inner))) { return ranges::iter_swap(__x._M_inner, __y._M_inner); } + friend _Iterator<!_Const>; friend _Sentinel<_Const>; }; @@ -2561,8 +2564,7 @@ namespace views _Vp _M_base = _Vp(); // XXX: _M_inner is "present only when !is_reference_v<_InnerRange>" - // Applied P3278 and made this field mutable. - [[no_unique_address]] mutable + [[no_unique_address]] conditional_t<!is_reference_v<_InnerRange>, all_view<_InnerRange>, __detail::_Empty> _M_inner; @@ -2595,8 +2597,10 @@ namespace views constexpr auto begin() { - return _Iterator<__detail::__simple_view<_Vp>>{*this, - ranges::begin(_M_base)}; + constexpr bool __use_const + = (__detail::__simple_view<_Vp> + && is_reference_v<range_reference_t<_Vp>>); + return _Iterator<__use_const>{*this, ranges::begin(_M_base)}; } constexpr auto @@ -2762,7 +2766,7 @@ namespace views constexpr _OuterIter(_OuterIter<!_Const> __i) requires _Const - && convertible_to<iterator_t<_Vp>, iterator_t<const _Vp>> + && convertible_to<iterator_t<_Vp>, iterator_t<_Base>> : _M_parent(__i._M_parent), _M_current(std::move(__i._M_current)) { } diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/filter.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/filter.cc index 834d10f7f91..84858798dda 100644 --- a/libstdc++-v3/testsuite/std/ranges/adaptors/filter.cc +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/filter.cc @@ -48,6 +48,8 @@ test01() static_assert(ranges::range<ranges::all_view<R>>); VERIFY( ranges::equal(v, (int[]){1,3,5}) ); VERIFY( ranges::equal(v | views::reverse, (int[]){5,3,1}) ); + VERIFY( v.pred()(3) == true ); + VERIFY( v.pred()(4) == false ); } void
On 18/02/20 14:31 -0500, Patrick Palka wrote: >On Tue, 18 Feb 2020, Patrick Palka wrote: > >> Among other changes, P1983R0 resolves LWG 3278 in a different way, so this patch >> also reverts the already-applied change of LWG 3278. >> >> The wording for US283 has already been applied it seems. >> >> libstdc++-v3/ChangeLog: >> >> P1983R0 Wording for GB301, US296, US292, US291, and US283 >> * include/std/ranges (filter_view::pred): New member function. >> (join_view::_Iterator::_Iterator): Remove now-redundant comment since >> P1983R0 fixes the highlighted issue in the same way. >> (join_view::_M_inner): Remove mutable specifier, effectively reverting >> the proposed wording changes of P3278. >> (join_view::begin): Refine the condition for when to return a const >> iterator. >> (split_view::_OuterIter::_OuterIter): Adjust constraints. >> * testsuite/std/ranges/adaptors/filter.cc: Test that filter_view::pred >> exists and works. > >Oops, this patch is incomplete, it breaks the join_view adaptor test due >to a missing friend, sorry about that. Here's the complete and tested >patch: OK for master, thanks.
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges index a04387a75c4..2f090dbd9fb 100644 --- a/libstdc++-v3/include/std/ranges +++ b/libstdc++-v3/include/std/ranges @@ -1556,6 +1556,10 @@ namespace views base() && { return std::move(_M_base); } + constexpr const _Pred& + pred() const + { return *_M_pred; } + constexpr _Iterator begin() { @@ -2421,8 +2425,6 @@ namespace views _Iterator() = default; - // XXX: had to change the type of __outer from iterator_t<_Vp> to - // iterator_t<_Base> here, a possible defect in the spec? constexpr _Iterator(_Parent& __parent, iterator_t<_Base> __outer) : _M_outer(std::move(__outer)), @@ -2560,9 +2562,7 @@ namespace views _Vp _M_base = _Vp(); - // XXX: _M_inner is "present only when !is_reference_v<_InnerRange>" - // Applied P3278 and made this field mutable. - [[no_unique_address]] mutable + [[no_unique_address]] conditional_t<!is_reference_v<_InnerRange>, all_view<_InnerRange>, __detail::_Empty> _M_inner; @@ -2595,8 +2595,10 @@ namespace views constexpr auto begin() { - return _Iterator<__detail::__simple_view<_Vp>>{*this, - ranges::begin(_M_base)}; + constexpr bool __use_const + = (__detail::__simple_view<_Vp> + && is_reference_v<range_reference_t<_Vp>>); + return _Iterator<__use_const>{*this, ranges::begin(_M_base)}; } constexpr auto @@ -2762,7 +2764,7 @@ namespace views constexpr _OuterIter(_OuterIter<!_Const> __i) requires _Const - && convertible_to<iterator_t<_Vp>, iterator_t<const _Vp>> + && convertible_to<iterator_t<_Vp>, iterator_t<_Base>> : _M_parent(__i._M_parent), _M_current(std::move(__i._M_current)) { } diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/filter.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/filter.cc index 834d10f7f91..84858798dda 100644 --- a/libstdc++-v3/testsuite/std/ranges/adaptors/filter.cc +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/filter.cc @@ -48,6 +48,8 @@ test01() static_assert(ranges::range<ranges::all_view<R>>); VERIFY( ranges::equal(v, (int[]){1,3,5}) ); VERIFY( ranges::equal(v | views::reverse, (int[]){5,3,1}) ); + VERIFY( v.pred()(3) == true ); + VERIFY( v.pred()(4) == false ); } void