libstdc++: Fix constraints for drop_view::begin() const [LWG 3482]
authorJonathan Wakely <jwakely@redhat.com>
Mon, 21 Sep 2020 22:43:25 +0000 (23:43 +0100)
committerJonathan Wakely <jwakely@redhat.com>
Mon, 21 Sep 2020 22:43:25 +0000 (23:43 +0100)
libstdc++-v3/ChangeLog:

* include/std/ranges (drop_view::begin()): Adjust constraints
to match the correct condition for O(1) ranges::next (LWG 3482).
* testsuite/std/ranges/adaptors/drop.cc: Check that iterator is
cached for non-sized_range.

libstdc++-v3/include/std/ranges
libstdc++-v3/testsuite/std/ranges/adaptors/drop.cc

index 005e89f94b2fa282592c82128d8d6aeec122ee28..1bf894dd57015232037f83f8e237557b6cf75db1 100644 (file)
@@ -2238,7 +2238,10 @@ namespace views
       _Vp _M_base = _Vp();
       range_difference_t<_Vp> _M_count = 0;
 
-      static constexpr bool _S_needs_cached_begin = !random_access_range<_Vp>;
+      // ranges::next(begin(base), count, end(base)) is O(1) if _Vp satisfies
+      // both random_access_range and sized_range. Otherwise, cache its result.
+      static constexpr bool _S_needs_cached_begin
+       = !(random_access_range<const _Vp> && sized_range<const _Vp>);
       [[no_unique_address]]
        __detail::__maybe_present_t<_S_needs_cached_begin,
                                    __detail::_CachedPosition<_Vp>>
@@ -2260,9 +2263,12 @@ namespace views
       base() &&
       { return std::move(_M_base); }
 
+      // This overload is disabled for simple views with constant-time begin().
       constexpr auto
-      begin() requires (!(__detail::__simple_view<_Vp>
-                         && random_access_range<_Vp>))
+      begin()
+       requires (!(__detail::__simple_view<_Vp>
+                   && random_access_range<const _Vp>
+                   && sized_range<const _Vp>))
       {
        if constexpr (_S_needs_cached_begin)
          if (_M_cached_begin._M_has_value())
@@ -2275,8 +2281,11 @@ namespace views
        return __it;
       }
 
+      // _GLIBCXX_RESOLVE_LIB_DEFECTS
+      // 3482. drop_view's const begin should additionally require sized_range
       constexpr auto
-      begin() const requires random_access_range<const _Vp>
+      begin() const
+       requires random_access_range<const _Vp> && sized_range<const _Vp>
       {
        return ranges::next(ranges::begin(_M_base), _M_count,
                            ranges::end(_M_base));
index 3c82caea772d83e87a35f8277744018c63568323..5fe94b6750732c1d93f6fd4ad9b2f3eece975915 100644 (file)
@@ -26,6 +26,7 @@
 using __gnu_test::test_range;
 using __gnu_test::forward_iterator_wrapper;
 using __gnu_test::bidirectional_iterator_wrapper;
+using __gnu_test::random_access_iterator_wrapper;
 
 namespace ranges = std::ranges;
 namespace views = ranges::views;
@@ -123,8 +124,52 @@ struct test_wrapper : forward_iterator_wrapper<T>
     forward_iterator_wrapper<T>::operator++();
     return *this;
   }
+};
 
-  test_wrapper
+void
+test07()
+{
+  int x[] = {1,2,3,4,5};
+  test_range<int, test_wrapper> rx(x);
+  auto v = rx | views::drop(3);
+  VERIFY( test_wrapper<int>::increment_count == 0 );
+  (void) v.begin();
+  VERIFY( test_wrapper<int>::increment_count == 3 );
+  (void) v.begin();
+  VERIFY( test_wrapper<int>::increment_count == 3 );
+  VERIFY( ranges::equal(v, (int[]){4,5}) );
+  VERIFY( test_wrapper<int>::increment_count == 5 );
+  VERIFY( ranges::equal(v, (int[]){4,5}) );
+  VERIFY( test_wrapper<int>::increment_count == 7 );
+}
+
+template<typename T>
+struct ra_test_wrapper : random_access_iterator_wrapper<T>
+{
+  static inline int increment_count = 0;
+
+  using random_access_iterator_wrapper<T>::random_access_iterator_wrapper;
+
+  ra_test_wrapper() : random_access_iterator_wrapper<T>()
+  { }
+
+  ra_test_wrapper
+  operator++(int)
+  {
+    auto tmp = *this;
+    ++*this;
+    return tmp;
+  }
+
+  ra_test_wrapper&
+  operator++()
+  {
+    ++increment_count;
+    random_access_iterator_wrapper<T>::operator++();
+    return *this;
+  }
+
+  ra_test_wrapper
   operator--(int)
   {
     auto tmp = *this;
@@ -132,23 +177,85 @@ struct test_wrapper : forward_iterator_wrapper<T>
     return tmp;
   }
 
-  test_wrapper&
+  ra_test_wrapper&
   operator--()
   {
-    forward_iterator_wrapper<T>::operator--();
+    random_access_iterator_wrapper<T>::operator--();
+    return *this;
+  }
+
+  ra_test_wrapper&
+  operator+=(std::ptrdiff_t n)
+  {
+    random_access_iterator_wrapper<T>::operator+=(n);
     return *this;
   }
+
+  ra_test_wrapper&
+  operator-=(std::ptrdiff_t n)
+  { return *this += -n; }
+
+  ra_test_wrapper
+  operator+(std::ptrdiff_t n) const
+  {
+    auto tmp = *this;
+    return tmp += n;
+  }
+
+  ra_test_wrapper
+  operator-(std::ptrdiff_t n) const
+  {
+    auto tmp = *this;
+    return tmp -= n;
+  }
+
+  std::ptrdiff_t
+  operator-(const ra_test_wrapper& it) const
+  {
+    return static_cast<const random_access_iterator_wrapper<T>&>(*this) - it;
+  }
+
+  friend ra_test_wrapper
+  operator+(std::ptrdiff_t n, const ra_test_wrapper& it)
+  { return it + n; }
 };
 
 void
-test07()
+test08()
 {
-  int x[] = {1,2,3,4,5};
-  test_range<int, test_wrapper> rx(x);
-  auto v = rx | views::drop(3);
-  VERIFY( ranges::equal(v, (int[]){4,5}) );
-  VERIFY( ranges::equal(v, (int[]){4,5}) );
-  VERIFY( test_wrapper<int>::increment_count == 7 );
+  // LWG 3482 - drop_view's const begin should additionally require sized_range
+
+  short a[10]{ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+  test_range<short, ra_test_wrapper> ra(a);
+  static_assert( ranges::random_access_range<decltype(ra)> );
+  ranges::subrange nonsized = {ra.begin(), std::unreachable_sentinel};
+  using Nonsized = decltype(nonsized);
+  static_assert( ranges::random_access_range<Nonsized> );
+  static_assert( ! ranges::sized_range<Nonsized> );
+  auto v1 = nonsized | views::drop(5);
+  VERIFY( ra_test_wrapper<short>::increment_count == 0 );
+  auto b1 = v1.begin();
+  VERIFY( ra_test_wrapper<short>::increment_count == 5 );
+  VERIFY( v1.begin() == b1 );
+  VERIFY( ra_test_wrapper<short>::increment_count == 5 );
+  VERIFY( *b1 == 5 );
+  VERIFY( *v1.begin() == 5 );
+  VERIFY( ra_test_wrapper<short>::increment_count == 5 );
+
+  long b[10]{ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+  test_range<long, ra_test_wrapper> rb(b);
+  ranges::subrange sized = {rb.begin(), rb.begin()+6};
+  using Sized = decltype(sized);
+  static_assert( ranges::random_access_range<Sized> );
+  static_assert( ranges::sized_range<Sized> );
+  auto v2 = sized | views::drop(6);
+  auto b2 = v2.begin();
+  VERIFY( ra_test_wrapper<long>::increment_count == 0 );
+  VERIFY( v2.begin() == b2 );
+  VERIFY( ra_test_wrapper<long>::increment_count == 0 );
+  VERIFY( *b2 == 6 );
+  VERIFY( *v2.begin() == 6 );
+  VERIFY( ra_test_wrapper<long>::increment_count == 0 );
 }
 
 int
@@ -161,4 +268,5 @@ main()
   test05();
   test06();
   test07();
+  test08();
 }