[committed] libstdc++: Fix view adaptors for mixed-const sentinels and iterators (PR 95322)

Message ID 20200527210921.GA3030055@redhat.com
State New
Headers show
Series
  • [committed] libstdc++: Fix view adaptors for mixed-const sentinels and iterators (PR 95322)
Related show

Commit Message

Kewen.Lin via Gcc-patches May 27, 2020, 9:09 p.m.
The bug report is that transform_view's sentinel<false> cannot be
compared to its iterator<true>.  The comparison is supposed to use
operator==(iterator<Const>, sentinel<Const>) after converting
sentinel<false> to sentinel<true>. However, the operator== is a hidden
friend so is not a candidate when comparing iterator<true> with
sentinel<false>. 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<true> and _Iterator<false>.
	(join_view::_Sentinel): Likewise.
	* testsuite/std/ranges/adaptors/95322.cc: New test.

Tested powerpc64le-linux, committed to master.

I intend to backport this to gcc-10 as well.
commit 6c2582c0406250c66e2eb3651f8e8638796b7f53
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed May 27 22:08:15 2020 +0100

    libstdc++: Fix view adaptors for mixed-const sentinels and iterators (PR 95322)
    
    The bug report is that transform_view's sentinel<false> cannot be
    compared to its iterator<true>.  The comparison is supposed to use
    operator==(iterator<Const>, sentinel<Const>) after converting
    sentinel<false> to sentinel<true>. However, the operator== is a hidden
    friend so is not a candidate when comparing iterator<true> with
    sentinel<false>. 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<true> and _Iterator<false>.
            (join_view::_Sentinel): Likewise.
            * testsuite/std/ranges/adaptors/95322.cc: New test.

Patch

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<!_Const>;
-	  friend _Sentinel<_Const>;
+	  template<bool> friend struct _Sentinel;
 	};
 
       template<bool _Const>
@@ -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<bool _Const2>
+	    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<bool _Const2>
+	    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); }
+	  template<bool _Const2>
+	    requires sentinel_for<sentinel_t<_Base>,
+		       iterator_t<__detail::__maybe_const_t<_Const2, _Vp>>>
+	    friend constexpr bool
+	    operator==(const _Iterator<_Const2>& __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<sentinel_t<_Base>, iterator_t<_Base>>
-	  { return -__y.__distance_from(__x); }
+	  template<bool _Const2>
+	    requires sized_sentinel_for<sentinel_t<_Base>,
+		       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); }
 
-	  friend constexpr range_difference_t<_Base>
-	  operator-(const _Sentinel& __y, const _Iterator<_Const>& __x)
-	    requires sized_sentinel_for<sentinel_t<_Base>, iterator_t<_Base>>
-	  { return __y.__distance_from(__x); }
+	  template<bool _Const2>
+	    requires sized_sentinel_for<sentinel_t<_Base>,
+		       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<!_Const>;
 	};
@@ -2571,7 +2580,7 @@  namespace views
 	  { return ranges::iter_swap(__x._M_inner, __y._M_inner); }
 
 	  friend _Iterator<!_Const>;
-	  friend _Sentinel<_Const>;
+	  template<bool> friend struct _Sentinel;
 	};
 
       template<bool _Const>
@@ -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<bool _Const2>
+	    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<bool _Const2>
+	    requires sentinel_for<sentinel_t<_Base>,
+		       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<!_Const>;
 	};
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
 // <http://www.gnu.org/licenses/>.
 
-// { dg-options "-std=gnu++2a" }
+// { dg-options "-std=gnu++20" }
 // { dg-do run { target c++2a } }
 
 #include <ranges>
-#include <list>
-#include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
 
-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<int> 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>;
+
+  int a[2]{1, 2};
+  int b[3]{3, 4, 5};
+  irange u[2]{ irange{a}, irange{b} };
+  test_forward_range<irange> 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();
 }