]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
libstdc++: Implement P2770R0 changes to join_view / join_with_view
authorPatrick Palka <ppalka@redhat.com>
Thu, 17 Aug 2023 16:56:32 +0000 (12:56 -0400)
committerPatrick Palka <ppalka@redhat.com>
Thu, 17 Aug 2023 16:56:32 +0000 (12:56 -0400)
This C++23 paper fixes an issue in these views when adapting a certain
kind of non-forward range, and we treat it as a DR against C++20.

Reviewed-by: Jonathan Wakely <jwakely@redhat.com>
libstdc++-v3/ChangeLog:

* include/bits/regex.h (regex_iterator::iterator_concept):
Define for C++20 as per P2770R0.
(regex_token_iterator::iterator_concept): Likewise.
* include/std/ranges (__detail::__as_lvalue): Define.
(join_view::_Iterator): Befriend join_view.
(join_view::_Iterator::_M_satisfy): Use _M_get_outer
instead of _M_outer.
(join_view::_Iterator::_M_get_outer): Define.
(join_view::_Iterator::_Iterator): Split constructor taking
_Parent argument into two as per P2770R0.  Remove constraint on
default constructor.
(join_view::_Iterator::_M_outer): Make this data member present
only when the underlying range is forward.
(join_view::_Iterator::operator++): Use _M_get_outer instead of
_M_outer.
(join_view::_Iterator::operator--): Use __as_lvalue helper.
(join_view::_Iterator::operator==): Adjust constraints as per
P2770R0.
(join_view::_Sentinel::__equal): Use _M_get_outer instead of
_M_outer.
(join_view::_M_outer): New data member when the underlying range
is non-forward.
(join_view::begin): Adjust definition as per P2770R0.
(join_view::end): Likewise.
(join_with_view::_M_outer_it): New data member when the
underlying range is non-forward.
(join_with_view::begin): Adjust definition as per P2770R0.
(join_with_view::end): Likewise.
(join_with_view::_Iterator::_M_outer_it): Make this data member
present only when the underlying range is forward.
(join_with_view::_Iterator::_M_get_outer): Define.
(join_with_view::_Iterator::_Iterator): Split constructor
taking _Parent argument into two as per P2770R0.  Remove
constraint on default constructor.
(join_with_view::_Iterator::_M_update_inner): Adjust definition
as per P2770R0.
(join_with_view::_Iterator::_M_get_inner): Likewise.
(join_with_view::_Iterator::_M_satisfy): Adjust calls to
_M_get_inner.  Use _M_get_outer instead of _M_outer_it.
(join_with_view::_Iterator::operator==): Adjust constraints
as per P2770R0.
(join_with_view::_Sentinel::operator==): Use _M_get_outer
instead of _M_outer_it.
* testsuite/std/ranges/adaptors/p2770r0.cc: New test.

libstdc++-v3/include/bits/regex.h
libstdc++-v3/include/std/ranges
libstdc++-v3/testsuite/std/ranges/adaptors/p2770r0.cc [new file with mode: 0644]

index 26ac6a21c31dbad727499dce3a610beb66dd96ff..2d30686872138922bc39d42d731a90de5b1ed930 100644 (file)
@@ -2740,6 +2740,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       typedef const value_type*                  pointer;
       typedef const value_type&                  reference;
       typedef std::forward_iterator_tag          iterator_category;
+#if __cplusplus > 201703L
+      typedef std::input_iterator_tag    iterator_concept;
+#endif
 
       /**
        * @brief Provides a singular iterator, useful for indicating
@@ -2869,6 +2872,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       typedef const value_type*                        pointer;
       typedef const value_type&                        reference;
       typedef std::forward_iterator_tag                iterator_category;
+#if __cplusplus > 201703L
+      typedef std::input_iterator_tag          iterator_concept;
+#endif
 
     public:
       /**
index ffa3e97ef94437b30a6e476fed792ab251455ab3..3477323d8711088ded990202c9fe5a58791da4d1 100644 (file)
@@ -2738,6 +2738,14 @@ namespace views::__adaptor
     inline constexpr _DropWhile drop_while;
   } // namespace views
 
+  namespace __detail
+  {
+    template<typename _Tp>
+      constexpr _Tp&
+      __as_lvalue(_Tp&& __t)
+      { return static_cast<_Tp&>(__t); }
+  } // namespace __detail
+
   template<input_range _Vp>
     requires view<_Vp> && input_range<range_reference_t<_Vp>>
     class join_view : public view_interface<join_view<_Vp>>
@@ -2800,6 +2808,8 @@ namespace views::__adaptor
          using _Parent = __detail::__maybe_const_t<_Const, join_view>;
          using _Base = join_view::_Base<_Const>;
 
+         friend join_view;
+
          static constexpr bool _S_ref_is_glvalue
            = join_view::_S_ref_is_glvalue<_Const>;
 
@@ -2813,9 +2823,10 @@ namespace views::__adaptor
                return _M_parent->_M_inner._M_emplace_deref(__x);
            };
 
-           for (; _M_outer != ranges::end(_M_parent->_M_base); ++_M_outer)
+           _Outer_iter& __outer = _M_get_outer();
+           for (; __outer != ranges::end(_M_parent->_M_base); ++__outer)
              {
-               auto&& __inner = __update_inner(_M_outer);
+               auto&& __inner = __update_inner(__outer);
                _M_inner = ranges::begin(__inner);
                if (_M_inner != ranges::end(__inner))
                  return;
@@ -2844,7 +2855,36 @@ namespace views::__adaptor
          using _Outer_iter = join_view::_Outer_iter<_Const>;
          using _Inner_iter = join_view::_Inner_iter<_Const>;
 
-         _Outer_iter _M_outer = _Outer_iter();
+         constexpr _Outer_iter&
+         _M_get_outer()
+         {
+           if constexpr (forward_range<_Base>)
+             return _M_outer;
+           else
+             return *_M_parent->_M_outer;
+         }
+
+         constexpr const _Outer_iter&
+         _M_get_outer() const
+         {
+           if constexpr (forward_range<_Base>)
+             return _M_outer;
+           else
+             return *_M_parent->_M_outer;
+         }
+
+         constexpr
+         _Iterator(_Parent* __parent, _Outer_iter __outer) requires forward_range<_Base>
+           : _M_outer(std::move(__outer)), _M_parent(__parent)
+         { _M_satisfy(); }
+
+         constexpr explicit
+         _Iterator(_Parent* __parent) requires (!forward_range<_Base>)
+           : _M_parent(__parent)
+         { _M_satisfy(); }
+
+         [[no_unique_address]]
+           __detail::__maybe_present_t<forward_range<_Base>, _Outer_iter> _M_outer;
          optional<_Inner_iter> _M_inner;
          _Parent* _M_parent = nullptr;
 
@@ -2856,13 +2896,7 @@ namespace views::__adaptor
            = common_type_t<range_difference_t<_Base>,
                            range_difference_t<range_reference_t<_Base>>>;
 
-         _Iterator() requires default_initializable<_Outer_iter> = default;
-
-         constexpr
-         _Iterator(_Parent* __parent, _Outer_iter __outer)
-           : _M_outer(std::move(__outer)),
-             _M_parent(__parent)
-         { _M_satisfy(); }
+         _Iterator() = default;
 
          constexpr
          _Iterator(_Iterator<!_Const> __i)
@@ -2890,13 +2924,13 @@ namespace views::__adaptor
          {
            auto&& __inner_range = [this] () -> auto&& {
              if constexpr (_S_ref_is_glvalue)
-               return *_M_outer;
+               return *_M_get_outer();
              else
                return *_M_parent->_M_inner;
            }();
            if (++*_M_inner == ranges::end(__inner_range))
              {
-               ++_M_outer;
+               ++_M_get_outer();
                _M_satisfy();
              }
            return *this;
@@ -2923,9 +2957,9 @@ namespace views::__adaptor
              && common_range<range_reference_t<_Base>>
          {
            if (_M_outer == ranges::end(_M_parent->_M_base))
-             _M_inner = ranges::end(*--_M_outer);
-           while (*_M_inner == ranges::begin(*_M_outer))
-             *_M_inner = ranges::end(*--_M_outer);
+             _M_inner = ranges::end(__detail::__as_lvalue(*--_M_outer));
+           while (*_M_inner == ranges::begin(__detail::__as_lvalue(*_M_outer)))
+             *_M_inner = ranges::end(__detail::__as_lvalue(*--_M_outer));
            --*_M_inner;
            return *this;
          }
@@ -2944,7 +2978,7 @@ namespace views::__adaptor
          friend constexpr bool
          operator==(const _Iterator& __x, const _Iterator& __y)
            requires _S_ref_is_glvalue
-             && equality_comparable<_Outer_iter>
+             && forward_range<_Base>
              && equality_comparable<_Inner_iter>
          {
            return (__x._M_outer == __y._M_outer
@@ -2976,7 +3010,7 @@ namespace views::__adaptor
          template<bool _Const2>
            constexpr bool
            __equal(const _Iterator<_Const2>& __i) const
-           { return __i._M_outer == _M_end; }
+           { return __i._M_get_outer() == _M_end; }
 
          sentinel_t<_Base> _M_end = sentinel_t<_Base>();
 
@@ -3005,6 +3039,9 @@ namespace views::__adaptor
        };
 
       _Vp _M_base = _Vp();
+      [[no_unique_address]]
+       __detail::__maybe_present_t<!forward_range<_Vp>,
+         __detail::__non_propagating_cache<iterator_t<_Vp>>> _M_outer;
       [[no_unique_address]]
        __detail::__non_propagating_cache<remove_cv_t<_InnerRange>> _M_inner;
 
@@ -3027,16 +3064,25 @@ namespace views::__adaptor
       constexpr auto
       begin()
       {
-       constexpr bool __use_const
-         = (__detail::__simple_view<_Vp>
-            && is_reference_v<range_reference_t<_Vp>>);
-       return _Iterator<__use_const>{this, ranges::begin(_M_base)};
+       if constexpr (forward_range<_Vp>)
+         {
+           constexpr bool __use_const
+             = (__detail::__simple_view<_Vp>
+                && is_reference_v<range_reference_t<_Vp>>);
+           return _Iterator<__use_const>{this, ranges::begin(_M_base)};
+         }
+       else
+         {
+           _M_outer = ranges::begin(_M_base);
+           return _Iterator<false>{this};
+         }
       }
 
       constexpr auto
       begin() const
-       requires input_range<const _Vp>
+       requires forward_range<const _Vp>
          && is_reference_v<range_reference_t<const _Vp>>
+         && input_range<range_reference_t<const _Vp>>
       {
        return _Iterator<true>{this, ranges::begin(_M_base)};
       }
@@ -3055,11 +3101,11 @@ namespace views::__adaptor
 
       constexpr auto
       end() const
-       requires input_range<const _Vp>
+       requires forward_range<const _Vp>
          && is_reference_v<range_reference_t<const _Vp>>
+         && input_range<range_reference_t<const _Vp>>
       {
-       if constexpr (forward_range<const _Vp>
-                     && is_reference_v<range_reference_t<const _Vp>>
+       if constexpr (is_reference_v<range_reference_t<const _Vp>>
                      && forward_range<range_reference_t<const _Vp>>
                      && common_range<const _Vp>
                      && common_range<range_reference_t<const _Vp>>)
@@ -6978,6 +7024,9 @@ namespace views::__adaptor
     using _InnerRange = range_reference_t<_Vp>;
 
     _Vp _M_base = _Vp();
+    [[no_unique_address]]
+      __detail::__maybe_present_t<!forward_range<_Vp>,
+       __detail::__non_propagating_cache<iterator_t<_Vp>>> _M_outer_it;
     __detail::__non_propagating_cache<remove_cv_t<_InnerRange>> _M_inner;
     _Pattern _M_pattern = _Pattern();
 
@@ -7065,16 +7114,25 @@ namespace views::__adaptor
     constexpr auto
     begin()
     {
-      constexpr bool __use_const = is_reference_v<_InnerRange>
-       && __detail::__simple_view<_Vp> && __detail::__simple_view<_Pattern>;
-      return _Iterator<__use_const>{*this, ranges::begin(_M_base)};
+      if constexpr (forward_range<_Vp>)
+       {
+         constexpr bool __use_const = is_reference_v<_InnerRange>
+           && __detail::__simple_view<_Vp> && __detail::__simple_view<_Pattern>;
+         return _Iterator<__use_const>{*this, ranges::begin(_M_base)};
+       }
+      else
+       {
+         _M_outer_it = ranges::begin(_M_base);
+         return _Iterator<false>{*this};
+       }
     }
 
     constexpr auto
     begin() const
-      requires input_range<const _Vp>
+      requires forward_range<const _Vp>
        && forward_range<const _Pattern>
        && is_reference_v<range_reference_t<const _Vp>>
+       && input_range<range_reference_t<const _Vp>>
     { return _Iterator<true>{*this, ranges::begin(_M_base)}; }
 
     constexpr auto
@@ -7092,13 +7150,13 @@ namespace views::__adaptor
 
     constexpr auto
     end() const
-      requires input_range<const _Vp>
+      requires forward_range<const _Vp>
        && forward_range<const _Pattern>
        && is_reference_v<range_reference_t<const _Vp>>
+       && input_range<range_reference_t<const _Vp>>
     {
       using _InnerConstRange = range_reference_t<const _Vp>;
-      if constexpr (forward_range<const _Vp>
-                   && forward_range<_InnerConstRange>
+      if constexpr (forward_range<_InnerConstRange>
                    && common_range<const _Vp>
                    && common_range<_InnerConstRange>)
         return _Iterator<true>{*this, ranges::end(_M_base)};
@@ -7135,35 +7193,69 @@ namespace views::__adaptor
     static constexpr bool _S_ref_is_glvalue = join_with_view::_S_ref_is_glvalue<_Const>;
 
     _Parent* _M_parent = nullptr;
-    _OuterIter _M_outer_it = _OuterIter();
+    [[no_unique_address]]
+      __detail::__maybe_present_t<forward_range<_Base>, _OuterIter> _M_outer_it;
     variant<_PatternIter, _InnerIter> _M_inner_it;
 
+    constexpr _OuterIter&
+    _M_get_outer()
+    {
+      if constexpr (forward_range<_Base>)
+       return _M_outer_it;
+      else
+       return *_M_parent->_M_outer_it;
+    }
+
+    constexpr const _OuterIter&
+    _M_get_outer() const
+    {
+      if constexpr (forward_range<_Base>)
+       return _M_outer_it;
+      else
+       return *_M_parent->_M_outer_it;
+    }
+
     constexpr
-    _Iterator(_Parent& __parent, iterator_t<_Base> __outer)
+    _Iterator(_Parent& __parent, _OuterIter __outer)
+      requires forward_range<_Base>
     : _M_parent(std::__addressof(__parent)), _M_outer_it(std::move(__outer))
     {
-      if (_M_outer_it != ranges::end(_M_parent->_M_base))
+      if (_M_get_outer() != ranges::end(_M_parent->_M_base))
+       {
+         auto&& __inner = _M_update_inner();
+         _M_inner_it.template emplace<1>(ranges::begin(__inner));
+         _M_satisfy();
+       }
+    }
+
+    constexpr
+    _Iterator(_Parent& __parent)
+      requires (!forward_range<_Base>)
+    : _M_parent(std::__addressof(__parent))
+    {
+      if (_M_get_outer() != ranges::end(_M_parent->_M_base))
        {
-         auto&& __inner = _M_update_inner(_M_outer_it);
+         auto&& __inner = _M_update_inner();
          _M_inner_it.template emplace<1>(ranges::begin(__inner));
          _M_satisfy();
        }
     }
 
-    constexpr auto&&
-    _M_update_inner(const _OuterIter& __x)
+    constexpr auto&
+    _M_update_inner()
     {
+      _OuterIter& __outer = _M_get_outer();
       if constexpr (_S_ref_is_glvalue)
-       return *__x;
+       return __detail::__as_lvalue(*__outer);
       else
-       return _M_parent->_M_inner._M_emplace_deref(__x);
+       return _M_parent->_M_inner._M_emplace_deref(__outer);
     }
 
-    constexpr auto&&
-    _M_get_inner(const _OuterIter& __x)
+    constexpr auto&
+    _M_get_inner()
     {
       if constexpr (_S_ref_is_glvalue)
-       return *__x;
+       return __detail::__as_lvalue(*_M_get_outer());
       else
        return *_M_parent->_M_inner;
     }
@@ -7178,16 +7270,16 @@ namespace views::__adaptor
              if (std::get<0>(_M_inner_it) != ranges::end(_M_parent->_M_pattern))
                break;
 
-             auto&& __inner = _M_update_inner(_M_outer_it);
+             auto&& __inner = _M_update_inner();
              _M_inner_it.template emplace<1>(ranges::begin(__inner));
            }
          else
            {
-             auto&& __inner = _M_get_inner(_M_outer_it);
+             auto&& __inner = _M_get_inner();
              if (std::get<1>(_M_inner_it) != ranges::end(__inner))
                break;
 
-             if (++_M_outer_it == ranges::end(_M_parent->_M_base))
+             if (++_M_get_outer() == ranges::end(_M_parent->_M_base))
                {
                  if constexpr (_S_ref_is_glvalue)
                    _M_inner_it.template emplace<0>();
@@ -7226,7 +7318,7 @@ namespace views::__adaptor
                                          iter_difference_t<_InnerIter>,
                                          iter_difference_t<_PatternIter>>;
 
-    _Iterator() requires default_initializable<_OuterIter> = default;
+    _Iterator() = default;
 
     constexpr
     _Iterator(_Iterator<!_Const> __i)
@@ -7336,7 +7428,7 @@ namespace views::__adaptor
     friend constexpr bool
     operator==(const _Iterator& __x, const _Iterator& __y)
       requires _S_ref_is_glvalue
-       && equality_comparable<_OuterIter> && equality_comparable<_InnerIter>
+       && forward_range<_Base> && equality_comparable<_InnerIter>
     { return __x._M_outer_it == __y._M_outer_it && __x._M_inner_it ==__y._M_inner_it; }
 
     friend constexpr common_reference_t<iter_rvalue_reference_t<_InnerIter>,
@@ -7403,7 +7495,7 @@ namespace views::__adaptor
                            iterator_t<__detail::__maybe_const_t<_OtherConst, _Vp>>>
     friend constexpr bool
     operator==(const _Iterator<_OtherConst>& __x, const _Sentinel& __y)
-    { return __x._M_outer_it == __y._M_end; }
+    { return __x._M_get_outer() == __y._M_end; }
   };
 
   namespace views
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/p2770r0.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/p2770r0.cc
new file mode 100644 (file)
index 0000000..15d71b2
--- /dev/null
@@ -0,0 +1,110 @@
+// { dg-options "-std=gnu++20" }
+// { dg-do run { target c++20 } }
+
+#include <ranges>
+#include <algorithm>
+#include <regex>
+#include <string_view>
+#include <testsuite_hooks.h>
+
+namespace ranges = std::ranges;
+namespace views = std::views;
+
+void
+test01()
+{
+  // Test case from LWG 3698
+  char const text[] = "Hello";
+  std::regex regex{"[a-z]"};
+
+  auto lower
+    = ranges::subrange(std::cregex_iterator(ranges::begin(text),
+                                           ranges::end(text),
+                                           regex),
+                      std::cregex_iterator{})
+    | views::join
+    | views::transform([](auto const& sm) {
+      return std::string_view(sm.first, sm.second);
+    });
+
+  VERIFY( ranges::equal(lower, (std::string_view[]){"e", "l", "l", "o"}));
+}
+
+void
+test02()
+{
+#if __cpp_lib_ranges_join_with
+  // Analogous test case from LWG 3698 for join_with_view
+  char const text[] = "Hello";
+  std::regex regex{"[a-z]"};
+
+  auto lower
+    = ranges::subrange(std::cregex_iterator(ranges::begin(text),
+                                           ranges::end(text),
+                                           regex),
+                      std::cregex_iterator{})
+    | views::join_with(views::empty<std::sub_match<const char*>>)
+    | views::transform([](auto const& sm) {
+      return std::string_view(sm.first, sm.second);
+    });
+
+  VERIFY( ranges::equal(lower, (std::string_view[]){"e", "l", "l", "o"}));
+#endif
+}
+
+void
+test03()
+{
+  // Test case from LWG 3700
+  auto r = views::iota(0, 5) | views::split(1);
+  auto s = views::single(r);
+  auto j = s | views::join;
+  auto f = j.front();
+}
+
+void
+test04()
+{
+#if __cpp_lib_ranges_join_with
+  // Analogous test case from LWG 3700 for join_with_view
+  auto r = views::iota(0, 5) | views::split(1);
+  auto s = views::single(r);
+  auto j = s | views::join_with(views::empty<ranges::range_value_t<decltype(r)>>);
+  auto f = j.front();
+#endif
+}
+
+void
+test05()
+{
+  // Test case from LWG 3791
+  std::vector<std::vector<int>> v = {{1}};
+  auto r = v
+    | views::transform([](auto& x) -> auto&& { return std::move(x); })
+    | views::join;
+  auto e = --r.end();
+}
+
+void
+test06()
+{
+#if __cpp_lib_ranges_join_with
+  // Analogous test case from LWG 3791 for join_with_view
+  std::vector<std::vector<int>> v = {{1}};
+  auto r = v
+    | views::transform([](auto& x) -> auto&& { return std::move(x); })
+    | views::join_with(views::empty<int>);
+  auto e = --r.end();
+#endif
+}
+
+int
+main()
+{
+  test01();
+  test02();
+  test03();
+  test04();
+  test05();
+  test06();
+}