From 6c2582c0406250c66e2eb3651f8e8638796b7f53 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Wed, 27 May 2020 22:08:15 +0100 Subject: [PATCH] libstdc++: Fix view adaptors for mixed-const sentinels and iterators (PR 95322) The bug report is that transform_view's sentinel cannot be compared to its iterator. The comparison is supposed to use operator==(iterator, sentinel) after converting sentinel to sentinel. However, the operator== is a hidden friend so is not a candidate when comparing iterator with sentinel. The required conversion would only happen if we'd found the operator, but we can't find the operator until after the conversion happens. A new LWG issue has been reported, but not yet assigned a number. The solution suggested by Casey Carter is to make the hidden friends of the sentinel types work with iterators of any const-ness, so that no conversions are required. Patrick Palka observed that join_view has a similar problem and a similar fix is used for its sentinel. PR libstdc++/95322 * include/std/ranges (transform_view::_Sentinel): Allow hidden friends to work with _Iterator and _Iterator. (join_view::_Sentinel): Likewise. * testsuite/std/ranges/adaptors/95322.cc: New test. --- libstdc++-v3/include/std/ranges | 67 +++++++++++-------- .../testsuite/std/ranges/adaptors/95322.cc | 48 +++++++------ 2 files changed, 68 insertions(+), 47 deletions(-) diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges index 0c602c7200f..b8023e67c9f 100644 --- a/libstdc++-v3/include/std/ranges +++ b/libstdc++-v3/include/std/ranges @@ -1853,7 +1853,7 @@ namespace views { return ranges::iter_swap(__x._M_current, __y._M_current); } friend _Iterator; - friend _Sentinel<_Const>; + template friend struct _Sentinel; }; template @@ -1863,13 +1863,15 @@ namespace views using _Parent = __detail::__maybe_const_t<_Const, transform_view>; using _Base = __detail::__maybe_const_t<_Const, _Vp>; - constexpr range_difference_t<_Base> - __distance_from(const _Iterator<_Const>& __i) const - { return _M_end - __i._M_current; } + template + constexpr range_difference_t<_Base> + __distance_from(const _Iterator<_Const2>& __i) const + { return _M_end - __i._M_current; } - constexpr bool - __equal(const _Iterator<_Const>& __i) const - { return __i._M_current == _M_end; } + template + constexpr bool + __equal(const _Iterator<_Const2>& __i) const + { return __i._M_current == _M_end; } sentinel_t<_Base> _M_end = sentinel_t<_Base>(); @@ -1892,19 +1894,26 @@ namespace views base() const { return _M_end; } - friend constexpr bool - operator==(const _Iterator<_Const>& __x, const _Sentinel& __y) - { return __y.__equal(__x); } - - friend constexpr range_difference_t<_Base> - operator-(const _Iterator<_Const>& __x, const _Sentinel& __y) - requires sized_sentinel_for, iterator_t<_Base>> - { return -__y.__distance_from(__x); } - - friend constexpr range_difference_t<_Base> - operator-(const _Sentinel& __y, const _Iterator<_Const>& __x) - requires sized_sentinel_for, iterator_t<_Base>> - { return __y.__distance_from(__x); } + template + requires sentinel_for, + iterator_t<__detail::__maybe_const_t<_Const2, _Vp>>> + friend constexpr bool + operator==(const _Iterator<_Const2>& __x, const _Sentinel& __y) + { return __y.__equal(__x); } + + template + requires sized_sentinel_for, + iterator_t<__detail::__maybe_const_t<_Const2, _Vp>>> + friend constexpr range_difference_t<_Base> + operator-(const _Iterator<_Const2>& __x, const _Sentinel& __y) + { return -__y.__distance_from(__x); } + + template + requires sized_sentinel_for, + iterator_t<__detail::__maybe_const_t<_Const2, _Vp>>> + friend constexpr range_difference_t<_Base> + operator-(const _Sentinel& __y, const _Iterator<_Const2>& __x) + { return __y.__distance_from(__x); } friend _Sentinel; }; @@ -2571,7 +2580,7 @@ namespace views { return ranges::iter_swap(__x._M_inner, __y._M_inner); } friend _Iterator; - friend _Sentinel<_Const>; + template friend struct _Sentinel; }; template @@ -2581,9 +2590,10 @@ namespace views using _Parent = __detail::__maybe_const_t<_Const, join_view>; using _Base = __detail::__maybe_const_t<_Const, _Vp>; - constexpr bool - __equal(const _Iterator<_Const>& __i) const - { return __i._M_outer == _M_end; } + template + constexpr bool + __equal(const _Iterator<_Const2>& __i) const + { return __i._M_outer == _M_end; } sentinel_t<_Base> _M_end = sentinel_t<_Base>(); @@ -2601,9 +2611,12 @@ namespace views : _M_end(std::move(__s._M_end)) { } - friend constexpr bool - operator==(const _Iterator<_Const>& __x, const _Sentinel& __y) - { return __y.__equal(__x); } + template + requires sentinel_for, + iterator_t<__detail::__maybe_const_t<_Const2, _Vp>>> + friend constexpr bool + operator==(const _Iterator<_Const2>& __x, const _Sentinel& __y) + { return __y.__equal(__x); } friend _Sentinel; }; diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/95322.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/95322.cc index 44619d6719a..9279d5520a8 100644 --- a/libstdc++-v3/testsuite/std/ranges/adaptors/95322.cc +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/95322.cc @@ -15,36 +15,44 @@ // with this library; see the file COPYING3. If not see // . -// { dg-options "-std=gnu++2a" } +// { dg-options "-std=gnu++20" } // { dg-do run { target c++2a } } #include -#include -#include +#include -namespace ranges = std::ranges; -namespace views = std::views; +using __gnu_test::test_forward_range; void test01() { - std::list container{1, 2, 3, 4, 5}; - auto v = (container - | views::take(3) - | views::transform(std::negate{}) - | views::common); - auto i = ranges::cbegin(v); - VERIFY( *i == -1 ); - ++i; - VERIFY( *i == -2 ); - ++i; - VERIFY( *i == -3 ); - ++i; - VERIFY( i == ranges::end(v) ); + // PR libstdc++/95322 + int a[2]{1, 2}; + test_forward_range v{a}; + auto view1 = v | std::views::take(2); + auto view2 = view1 | std::views::transform(std::identity{}); + const bool eq = std::ranges::cbegin(view2) == std::ranges::end(view2); + VERIFY( !eq ); } -int -main() +void +test02() +{ + using irange = test_forward_range; + + int a[2]{1, 2}; + int b[3]{3, 4, 5}; + irange u[2]{ irange{a}, irange{b} }; + test_forward_range v{u}; + auto view = (std::views::counted(v.begin(), 2) + | std::views::transform(std::identity{}) + | std::views::join); + const bool eq = std::ranges::cbegin(view) == std::ranges::end(view); + VERIFY( !eq ); +} + +int main() { test01(); + test02(); } -- 2.30.2