public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-3294] libstdc++: Implement P2770R0 changes to join_view / join_with_view
@ 2023-08-17 16:56 Patrick Palka
  0 siblings, 0 replies; only message in thread
From: Patrick Palka @ 2023-08-17 16:56 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:bad357dd1b3402fd7991d1ca6eea2b055ba6d003

commit r14-3294-gbad357dd1b3402fd7991d1ca6eea2b055ba6d003
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Aug 17 12:56:32 2023 -0400

    libstdc++: Implement P2770R0 changes to join_view / join_with_view
    
    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.

Diff:
---
 libstdc++-v3/include/bits/regex.h                  |   6 +
 libstdc++-v3/include/std/ranges                    | 190 +++++++++++++++------
 .../testsuite/std/ranges/adaptors/p2770r0.cc       | 110 ++++++++++++
 3 files changed, 257 insertions(+), 49 deletions(-)

diff --git a/libstdc++-v3/include/bits/regex.h b/libstdc++-v3/include/bits/regex.h
index 26ac6a21c31d..2d3068687213 100644
--- a/libstdc++-v3/include/bits/regex.h
+++ b/libstdc++-v3/include/bits/regex.h
@@ -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:
       /**
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index ffa3e97ef944..3477323d8711 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -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
index 000000000000..15d71b2faa91
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/p2770r0.cc
@@ -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();
+}

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-08-17 16:56 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-17 16:56 [gcc r14-3294] libstdc++: Implement P2770R0 changes to join_view / join_with_view Patrick Palka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).