public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed] libstdc++: Fix value categories used by ranges access CPOs [PR 100824]
@ 2021-06-04 17:03 Jonathan Wakely
  2021-06-04 20:44 ` Jonathan Wakely
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Wakely @ 2021-06-04 17:03 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2693 bytes --]

The implementation of P2091R0 was incomplete, so that some range access
CPOs used perfect forwarding where they should not. This fixes it by
consistently operating on lvalues.

Some additional changes that are not necessary to fix the bug:

Modify the __as_const helper to simplify its usage. Instead of deducing
the value category from its argument, and requiring callers to forward
the argument as the correct category, add a non-deduced template
parameter which is used for the value category and accept the argument
as an lvalue. This means callers say __as_const<T>(t) instead of
__as_const(std::forward<T>(t)).

Always use an lvalue reference type as the template argument for the
_S_noexcept helpers, so that we only instantiate one specialization for
lvalues and rvalues of the same type.

Move some helper concepts and functions from namespace std::__detail
to ranges::__cust_access, to be consistent with the ranges::begin CPO.
This ensures that the __adl_begin concept and the _Begin::operator()
function are in the same namespace, so unqualified lookup is consistent
and the poison pills for begin are visible to both.

Simplified static assertions for arrays, because the expression a+0 is
already ill-formed for an array of incomplete type.

Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

libstdc++-v3/ChangeLog:

	PR libstdc++/100824
	* include/bits/iterator_concepts.h (__detail::__decay_copy)
	(__detail::__member_begin, __detail::__adl_begin): Move to
	namespace ranges::__cust_access.
	(__detail::__ranges_begin): Likewise, and rename to __begin.
	Remove redundant static assertion.
	* include/bits/ranges_base.h (_Begin, _End, _RBegin, _REnd):
	Use lvalue in noexcept specifier.
	(__as_const): Add non-deduced parameter for value category.
	(_CBegin, _CEnd, _CRBegin, _CREnd, _CData): Adjust uses of
	__as_const.
	(__member_size, __adl_size, __member_empty, __size0_empty):
	(__eq_iter_empty, __adl_data): Use lvalue objects in
	requirements.
	(__sentinel_size): Likewise. Add check for conversion to
	unsigned-like.
	(__member_data): Allow non-lvalue types to satisfy the concept,
	but use lvalue object in requirements.
	(_Size, _SSize): Remove forwarding to always use an lvalue.
	(_Data): Likewise. Add static assertion for arrays.
	* testsuite/std/ranges/access/cdata.cc: Adjust expected
	behaviour for rvalues. Add negative tests for ill-formed
	expressions.
	* testsuite/std/ranges/access/data.cc: Likewise.
	* testsuite/std/ranges/access/empty.cc: Adjust expected
	behaviour for rvalues.
	* testsuite/std/ranges/access/size.cc: Likewise.

Tested powerpc64le-linux. Committed to trunk.

I think this should be backported to gcc-11 too, and maybe gcc-10 too.



[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 26435 bytes --]

commit ee9548b36a7f17e8a63585b58f340c93dcba95d8
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 4 15:59:38 2021

    libstdc++: Fix value categories used by ranges access CPOs [PR 100824]
    
    The implementation of P2091R0 was incomplete, so that some range access
    CPOs used perfect forwarding where they should not. This fixes it by
    consistently operating on lvalues.
    
    Some additional changes that are not necessary to fix the bug:
    
    Modify the __as_const helper to simplify its usage. Instead of deducing
    the value category from its argument, and requiring callers to forward
    the argument as the correct category, add a non-deduced template
    parameter which is used for the value category and accept the argument
    as an lvalue. This means callers say __as_const<T>(t) instead of
    __as_const(std::forward<T>(t)).
    
    Always use an lvalue reference type as the template argument for the
    _S_noexcept helpers, so that we only instantiate one specialization for
    lvalues and rvalues of the same type.
    
    Move some helper concepts and functions from namespace std::__detail
    to ranges::__cust_access, to be consistent with the ranges::begin CPO.
    This ensures that the __adl_begin concept and the _Begin::operator()
    function are in the same namespace, so unqualified lookup is consistent
    and the poison pills for begin are visible to both.
    
    Simplified static assertions for arrays, because the expression a+0 is
    already ill-formed for an array of incomplete type.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/100824
            * include/bits/iterator_concepts.h (__detail::__decay_copy)
            (__detail::__member_begin, __detail::__adl_begin): Move to
            namespace ranges::__cust_access.
            (__detail::__ranges_begin): Likewise, and rename to __begin.
            Remove redundant static assertion.
            * include/bits/ranges_base.h (_Begin, _End, _RBegin, _REnd):
            Use lvalue in noexcept specifier.
            (__as_const): Add non-deduced parameter for value category.
            (_CBegin, _CEnd, _CRBegin, _CREnd, _CData): Adjust uses of
            __as_const.
            (__member_size, __adl_size, __member_empty, __size0_empty):
            (__eq_iter_empty, __adl_data): Use lvalue objects in
            requirements.
            (__sentinel_size): Likewise. Add check for conversion to
            unsigned-like.
            (__member_data): Allow non-lvalue types to satisfy the concept,
            but use lvalue object in requirements.
            (_Size, _SSize): Remove forwarding to always use an lvalue.
            (_Data): Likewise. Add static assertion for arrays.
            * testsuite/std/ranges/access/cdata.cc: Adjust expected
            behaviour for rvalues. Add negative tests for ill-formed
            expressions.
            * testsuite/std/ranges/access/data.cc: Likewise.
            * testsuite/std/ranges/access/empty.cc: Adjust expected
            behaviour for rvalues.
            * testsuite/std/ranges/access/size.cc: Likewise.

diff --git a/libstdc++-v3/include/bits/iterator_concepts.h b/libstdc++-v3/include/bits/iterator_concepts.h
index 8f97a5d8789..f4e94a6263b 100644
--- a/libstdc++-v3/include/bits/iterator_concepts.h
+++ b/libstdc++-v3/include/bits/iterator_concepts.h
@@ -925,8 +925,11 @@ namespace ranges
   struct default_sentinel_t { };
   inline constexpr default_sentinel_t default_sentinel{};
 
-  namespace __detail
+  // This is the namespace for [range.access] CPOs.
+  namespace ranges::__cust_access
   {
+    using std::__detail::__class_or_enum;
+
     template<typename _Tp>
       constexpr decay_t<_Tp>
       __decay_copy(_Tp&& __t)
@@ -936,9 +939,11 @@ namespace ranges
     template<typename _Tp>
       concept __member_begin = requires(_Tp& __t)
 	{
-	  { __detail::__decay_copy(__t.begin()) } -> input_or_output_iterator;
+	  { __cust_access::__decay_copy(__t.begin()) }
+	    -> input_or_output_iterator;
 	};
 
+    // Poison pills so that unqualified lookup doesn't find std::begin.
     void begin(auto&) = delete;
     void begin(const auto&) = delete;
 
@@ -946,7 +951,8 @@ namespace ranges
       concept __adl_begin = __class_or_enum<remove_reference_t<_Tp>>
 	&& requires(_Tp& __t)
 	{
-	  { __detail::__decay_copy(begin(__t)) } -> input_or_output_iterator;
+	  { __cust_access::__decay_copy(begin(__t)) }
+	    -> input_or_output_iterator;
 	};
 
     // Simplified version of std::ranges::begin that only supports lvalues,
@@ -954,24 +960,23 @@ namespace ranges
     template<typename _Tp>
       requires is_array_v<_Tp> || __member_begin<_Tp&> || __adl_begin<_Tp&>
       auto
-      __ranges_begin(_Tp& __t)
+      __begin(_Tp& __t)
       {
 	if constexpr (is_array_v<_Tp>)
-	  {
-	    static_assert(sizeof(remove_all_extents_t<_Tp>) != 0,
-			  "not array of incomplete type");
-	    return __t + 0;
-	  }
+	  return __t + 0;
 	else if constexpr (__member_begin<_Tp&>)
 	  return __t.begin();
 	else
 	  return begin(__t);
       }
+  } // namespace ranges::__cust_access
 
+  namespace __detail
+  {
     // Implementation of std::ranges::iterator_t, without using ranges::begin.
     template<typename _Tp>
       using __range_iter_t
-	= decltype(__detail::__ranges_begin(std::declval<_Tp&>()));
+	= decltype(ranges::__cust_access::__begin(std::declval<_Tp&>()));
 
   } // namespace __detail
 
diff --git a/libstdc++-v3/include/bits/ranges_base.h b/libstdc++-v3/include/bits/ranges_base.h
index 32d664f1a86..17a421a4927 100644
--- a/libstdc++-v3/include/bits/ranges_base.h
+++ b/libstdc++-v3/include/bits/ranges_base.h
@@ -89,10 +89,6 @@ namespace ranges
   namespace __cust_access
   {
     using std::ranges::__detail::__maybe_borrowed_range;
-    using std::__detail::__class_or_enum;
-    using std::__detail::__decay_copy;
-    using std::__detail::__member_begin;
-    using std::__detail::__adl_begin;
 
     struct _Begin
     {
@@ -114,13 +110,11 @@ namespace ranges
 	requires is_array_v<remove_reference_t<_Tp>> || __member_begin<_Tp>
 	  || __adl_begin<_Tp>
 	constexpr auto
-	operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp>())
+	operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
 	{
 	  if constexpr (is_array_v<remove_reference_t<_Tp>>)
 	    {
 	      static_assert(is_lvalue_reference_v<_Tp>);
-	      using _Up = remove_all_extents_t<remove_reference_t<_Tp>>;
-	      static_assert(sizeof(_Up) != 0, "not array of incomplete type");
 	      return __t + 0;
 	    }
 	  else if constexpr (__member_begin<_Tp>)
@@ -137,6 +131,7 @@ namespace ranges
 	    -> sentinel_for<decltype(_Begin{}(std::forward<_Tp>(__t)))>;
 	};
 
+    // Poison pills so that unqualified lookup doesn't find std::end.
     void end(auto&) = delete;
     void end(const auto&) = delete;
 
@@ -165,10 +160,10 @@ namespace ranges
 
     public:
       template<__maybe_borrowed_range _Tp>
-	requires is_bounded_array_v<remove_reference_t<_Tp>> || __member_end<_Tp>
-	|| __adl_end<_Tp>
+	requires is_bounded_array_v<remove_reference_t<_Tp>>
+	  || __member_end<_Tp> || __adl_end<_Tp>
 	constexpr auto
-	operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp>())
+	operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
 	{
 	  if constexpr (is_bounded_array_v<remove_reference_t<_Tp>>)
 	    {
@@ -182,12 +177,15 @@ namespace ranges
 	}
     };
 
-    template<typename _Tp>
+    // If _To is an lvalue-reference, return const _Tp&, otherwise const _Tp&&.
+    template<typename _To, typename _Tp>
       constexpr decltype(auto)
-      __as_const(_Tp&& __t) noexcept
+      __as_const(_Tp& __t) noexcept
       {
-	if constexpr (is_lvalue_reference_v<_Tp>)
-	  return static_cast<const remove_reference_t<_Tp>&>(__t);
+	static_assert(std::is_same_v<_To&, _Tp&>);
+
+	if constexpr (is_lvalue_reference_v<_To>)
+	  return const_cast<const _Tp&>(__t);
 	else
 	  return static_cast<const _Tp&&>(__t);
       }
@@ -197,10 +195,10 @@ namespace ranges
       template<typename _Tp>
 	constexpr auto
 	operator()(_Tp&& __e) const
-	noexcept(noexcept(_Begin{}(__cust_access::__as_const((_Tp&&)__e))))
-	requires requires { _Begin{}(__cust_access::__as_const((_Tp&&)__e)); }
+	noexcept(noexcept(_Begin{}(__cust_access::__as_const<_Tp>(__e))))
+	requires requires { _Begin{}(__cust_access::__as_const<_Tp>(__e)); }
 	{
-	  return _Begin{}(__cust_access::__as_const(std::forward<_Tp>(__e)));
+	  return _Begin{}(__cust_access::__as_const<_Tp>(__e));
 	}
     };
 
@@ -209,10 +207,10 @@ namespace ranges
       template<typename _Tp>
 	constexpr auto
 	operator()(_Tp&& __e) const
-	noexcept(noexcept(_End{}(__cust_access::__as_const((_Tp&&)__e))))
-	requires requires { _End{}(__cust_access::__as_const((_Tp&&)__e)); }
+	noexcept(noexcept(_End{}(__cust_access::__as_const<_Tp>(__e))))
+	requires requires { _End{}(__cust_access::__as_const<_Tp>(__e)); }
 	{
-	  return _End{}(__cust_access::__as_const(std::forward<_Tp>(__e)));
+	  return _End{}(__cust_access::__as_const<_Tp>(__e));
 	}
     };
 
@@ -268,7 +266,7 @@ namespace ranges
 	requires __member_rbegin<_Tp> || __adl_rbegin<_Tp> || __reversable<_Tp>
 	constexpr auto
 	operator()(_Tp&& __t) const
-	noexcept(_S_noexcept<_Tp>())
+	noexcept(_S_noexcept<_Tp&>())
 	{
 	  if constexpr (__member_rbegin<_Tp>)
 	    return __t.rbegin();
@@ -326,7 +324,7 @@ namespace ranges
 	requires __member_rend<_Tp> || __adl_rend<_Tp> || __reversable<_Tp>
 	constexpr auto
 	operator()(_Tp&& __t) const
-	noexcept(_S_noexcept<_Tp>())
+	noexcept(_S_noexcept<_Tp&>())
 	{
 	  if constexpr (__member_rend<_Tp>)
 	    return __t.rend();
@@ -342,10 +340,10 @@ namespace ranges
       template<typename _Tp>
 	constexpr auto
 	operator()(_Tp&& __e) const
-	noexcept(noexcept(_RBegin{}(__cust_access::__as_const((_Tp&&)__e))))
-	requires requires { _RBegin{}(__cust_access::__as_const((_Tp&&)__e)); }
+	noexcept(noexcept(_RBegin{}(__cust_access::__as_const<_Tp>(__e))))
+	requires requires { _RBegin{}(__cust_access::__as_const<_Tp>(__e)); }
 	{
-	  return _RBegin{}(__cust_access::__as_const(std::forward<_Tp>(__e)));
+	  return _RBegin{}(__cust_access::__as_const<_Tp>(__e));
 	}
     };
 
@@ -354,19 +352,18 @@ namespace ranges
       template<typename _Tp>
 	constexpr auto
 	operator()(_Tp&& __e) const
-	noexcept(noexcept(_REnd{}(__cust_access::__as_const((_Tp&&)__e))))
-	requires requires { _REnd{}(__cust_access::__as_const((_Tp&&)__e)); }
+	noexcept(noexcept(_REnd{}(__cust_access::__as_const<_Tp>(__e))))
+	requires requires { _REnd{}(__cust_access::__as_const<_Tp>(__e)); }
 	{
-	  return _REnd{}(__cust_access::__as_const(std::forward<_Tp>(__e)));
+	  return _REnd{}(__cust_access::__as_const<_Tp>(__e));
 	}
     };
 
     template<typename _Tp>
       concept __member_size = !disable_sized_range<remove_cvref_t<_Tp>>
-	&& requires(_Tp&& __t)
+	&& requires(_Tp& __t)
 	{
-	  { __decay_copy(std::forward<_Tp>(__t).size()) }
-	    -> __detail::__is_integer_like;
+	  { __decay_copy(__t.size()) } -> __detail::__is_integer_like;
 	};
 
     void size(auto&) = delete;
@@ -375,19 +372,19 @@ namespace ranges
     template<typename _Tp>
       concept __adl_size = __class_or_enum<remove_reference_t<_Tp>>
 	&& !disable_sized_range<remove_cvref_t<_Tp>>
-	&& requires(_Tp&& __t)
+	&& requires(_Tp& __t)
 	{
-	  { __decay_copy(size(std::forward<_Tp>(__t))) }
-	    -> __detail::__is_integer_like;
+	  { __decay_copy(size(__t)) } -> __detail::__is_integer_like;
 	};
 
     template<typename _Tp>
-      concept __sentinel_size = requires(_Tp&& __t)
+      concept __sentinel_size = requires(_Tp& __t)
 	{
-	  { _Begin{}(std::forward<_Tp>(__t)) } -> forward_iterator;
+	  { _Begin{}(__t) } -> forward_iterator;
 
-	  { _End{}(std::forward<_Tp>(__t)) }
-	    -> sized_sentinel_for<decltype(_Begin{}(std::forward<_Tp>(__t)))>;
+	  { _End{}(__t) } -> sized_sentinel_for<decltype(_Begin{}(__t))>;
+
+	  __detail::__to_unsigned_like(_End{}(__t) - _Begin{}(__t));
 	};
 
     struct _Size
@@ -400,12 +397,12 @@ namespace ranges
 	  if constexpr (is_bounded_array_v<remove_reference_t<_Tp>>)
 	    return true;
 	  else if constexpr (__member_size<_Tp>)
-	    return noexcept(__decay_copy(std::declval<_Tp>().size()));
+	    return noexcept(__decay_copy(std::declval<_Tp&>().size()));
 	  else if constexpr (__adl_size<_Tp>)
-	    return noexcept(__decay_copy(size(std::declval<_Tp>())));
+	    return noexcept(__decay_copy(size(std::declval<_Tp&>())));
 	  else if constexpr (__sentinel_size<_Tp>)
-	    return noexcept(_End{}(std::declval<_Tp>())
-			    - _Begin{}(std::declval<_Tp>()));
+	    return noexcept(_End{}(std::declval<_Tp&>())
+			    - _Begin{}(std::declval<_Tp&>()));
 	}
 
     public:
@@ -413,39 +410,30 @@ namespace ranges
 	requires is_bounded_array_v<remove_reference_t<_Tp>>
 	  || __member_size<_Tp> || __adl_size<_Tp> || __sentinel_size<_Tp>
 	constexpr auto
-	operator()(_Tp&& __e) const noexcept(_S_noexcept<_Tp>())
+	operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
 	{
 	  if constexpr (is_bounded_array_v<remove_reference_t<_Tp>>)
-	    {
-	      return extent_v<remove_reference_t<_Tp>>;
-	    }
+	    return extent_v<remove_reference_t<_Tp>>;
 	  else if constexpr (__member_size<_Tp>)
-	    return std::forward<_Tp>(__e).size();
+	    return __t.size();
 	  else if constexpr (__adl_size<_Tp>)
-	    return size(std::forward<_Tp>(__e));
+	    return size(__t);
 	  else if constexpr (__sentinel_size<_Tp>)
-	    return __detail::__to_unsigned_like(
-		_End{}(std::forward<_Tp>(__e))
-		- _Begin{}(std::forward<_Tp>(__e)));
+	    return __detail::__to_unsigned_like(_End{}(__t) - _Begin{}(__t));
 	}
     };
 
     struct _SSize
     {
       template<typename _Tp>
-	requires requires (_Tp&& __e)
-	  {
-	    _Begin{}(std::forward<_Tp>(__e));
-	    _Size{}(std::forward<_Tp>(__e));
-	  }
+	requires requires (_Tp& __t) { _Size{}(__t); }
 	constexpr auto
-	operator()(_Tp&& __e) const
-	noexcept(noexcept(_Size{}(std::forward<_Tp>(__e))))
+	operator()(_Tp&& __t) const noexcept(noexcept(_Size{}(__t)))
 	{
-	  using __iter_type = decltype(_Begin{}(std::forward<_Tp>(__e)));
+	  using __iter_type = decltype(_Begin{}(__t));
 	  using __diff_type = iter_difference_t<__iter_type>;
 	  using __gnu_cxx::__int_traits;
-	  auto __size = _Size{}(std::forward<_Tp>(__e));
+	  auto __size = _Size{}(__t);
 	  if constexpr (integral<__diff_type>)
 	    {
 	      if constexpr (__int_traits<__diff_type>::__digits
@@ -457,19 +445,17 @@ namespace ranges
     };
 
     template<typename _Tp>
-      concept __member_empty = requires(_Tp&& __t)
-	{ bool(std::forward<_Tp>(__t).empty()); };
+      concept __member_empty = requires(_Tp& __t) { bool(__t.empty()); };
 
     template<typename _Tp>
-      concept __size0_empty = requires(_Tp&& __t)
-	{ _Size{}(std::forward<_Tp>(__t)) == 0; };
+      concept __size0_empty = requires(_Tp& __t) { _Size{}(__t) == 0; };
 
     template<typename _Tp>
-      concept __eq_iter_empty = requires(_Tp&& __t)
+      concept __eq_iter_empty = requires(_Tp& __t)
 	{
-	  { _Begin{}(std::forward<_Tp>(__t)) } -> forward_iterator;
-	  bool(_Begin{}(std::forward<_Tp>(__t))
-	      == _End{}(std::forward<_Tp>(__t)));
+	  { _Begin{}(__t) } -> forward_iterator;
+
+	  bool(_Begin{}(__t) == _End{}(__t));
 	};
 
     struct _Empty
@@ -480,28 +466,27 @@ namespace ranges
 	_S_noexcept()
 	{
 	  if constexpr (__member_empty<_Tp>)
-	    return noexcept(std::declval<_Tp>().empty());
+	    return noexcept(std::declval<_Tp&>().empty());
 	  else if constexpr (__size0_empty<_Tp>)
-	    return noexcept(_Size{}(std::declval<_Tp>()) == 0);
+	    return noexcept(_Size{}(std::declval<_Tp&>()) == 0);
 	  else
-	    return noexcept(bool(_Begin{}(std::declval<_Tp>())
-		== _End{}(std::declval<_Tp>())));
+	    return noexcept(bool(_Begin{}(std::declval<_Tp&>())
+		== _End{}(std::declval<_Tp&>())));
 	}
 
     public:
       template<typename _Tp>
 	requires __member_empty<_Tp> || __size0_empty<_Tp>
-	|| __eq_iter_empty<_Tp>
+	  || __eq_iter_empty<_Tp>
 	constexpr bool
-	operator()(_Tp&& __e) const noexcept(_S_noexcept<_Tp>())
+	operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp&>())
 	{
 	  if constexpr (__member_empty<_Tp>)
-	    return bool(std::forward<_Tp>(__e).empty());
+	    return bool(__t.empty());
 	  else if constexpr (__size0_empty<_Tp>)
-	    return _Size{}(std::forward<_Tp>(__e)) == 0;
+	    return _Size{}(__t) == 0;
 	  else
-	    return bool(_Begin{}(std::forward<_Tp>(__e))
-		== _End{}(std::forward<_Tp>(__e)));
+	    return bool(_Begin{}(__t) == _End{}(__t));
 	}
     };
 
@@ -510,12 +495,12 @@ namespace ranges
 				    && is_object_v<remove_pointer_t<_Tp>>;
 
     template<typename _Tp>
-      concept __member_data = is_lvalue_reference_v<_Tp>
-	&& requires(_Tp __t) { { __t.data() } -> __pointer_to_object; };
+      concept __member_data
+	= requires(_Tp& __t) { { __t.data() } -> __pointer_to_object; };
 
     template<typename _Tp>
-      concept __begin_data = requires(_Tp&& __t)
-	{ { _Begin{}(std::forward<_Tp>(__t)) } -> contiguous_iterator; };
+      concept __begin_data = requires(_Tp& __t)
+	{ { _Begin{}(__t) } -> contiguous_iterator; };
 
     struct _Data
     {
@@ -525,21 +510,21 @@ namespace ranges
 	_S_noexcept()
 	{
 	  if constexpr (__member_data<_Tp>)
-	    return noexcept(__decay_copy(std::declval<_Tp>().data()));
+	    return noexcept(__decay_copy(std::declval<_Tp&>().data()));
 	  else
-	    return noexcept(_Begin{}(std::declval<_Tp>()));
+	    return noexcept(_Begin{}(std::declval<_Tp&>()));
 	}
 
     public:
       template<__maybe_borrowed_range _Tp>
 	requires __member_data<_Tp> || __begin_data<_Tp>
 	constexpr auto
-	operator()(_Tp&& __e) const noexcept(_S_noexcept<_Tp>())
+	operator()(_Tp&& __t) const noexcept(_S_noexcept<_Tp>())
 	{
 	  if constexpr (__member_data<_Tp>)
-	    return __e.data();
+	    return __t.data();
 	  else
-	    return std::to_address(_Begin{}(std::forward<_Tp>(__e)));
+	    return std::to_address(_Begin{}(__t));
 	}
     };
 
@@ -548,10 +533,10 @@ namespace ranges
       template<typename _Tp>
 	constexpr auto
 	operator()(_Tp&& __e) const
-	noexcept(noexcept(_Data{}(__cust_access::__as_const((_Tp&&)__e))))
-	requires requires { _Data{}(__cust_access::__as_const((_Tp&&)__e)); }
+	noexcept(noexcept(_Data{}(__cust_access::__as_const<_Tp>(__e))))
+	requires requires { _Data{}(__cust_access::__as_const<_Tp>(__e)); }
 	{
-	  return _Data{}(__cust_access::__as_const(std::forward<_Tp>(__e)));
+	  return _Data{}(__cust_access::__as_const<_Tp>(__e));
 	}
     };
 
diff --git a/libstdc++-v3/testsuite/std/ranges/access/cdata.cc b/libstdc++-v3/testsuite/std/ranges/access/cdata.cc
index bea27ccaed2..2dfb6830789 100644
--- a/libstdc++-v3/testsuite/std/ranges/access/cdata.cc
+++ b/libstdc++-v3/testsuite/std/ranges/access/cdata.cc
@@ -21,6 +21,10 @@
 #include <ranges>
 #include <testsuite_hooks.h>
 
+template<typename T>
+  concept has_cdata
+    = requires (T&& t) { std::ranges::cdata(std::forward<T>(t)); };
+
 void
 test01()
 {
@@ -31,12 +35,18 @@ test01()
     int* data() { return &j; }
     const R* data() const noexcept { return nullptr; }
   };
+  static_assert( has_cdata<R&> );
+  static_assert( has_cdata<const R&> );
   R r;
   const R& c = r;
   VERIFY( std::ranges::cdata(r) == (R*)nullptr );
   static_assert( noexcept(std::ranges::cdata(r)) );
   VERIFY( std::ranges::cdata(c) == (R*)nullptr );
   static_assert( noexcept(std::ranges::cdata(c)) );
+
+  // not lvalues and not borrowed ranges
+  static_assert( !has_cdata<R> );
+  static_assert( !has_cdata<const R> );
 }
 
 void
@@ -44,28 +54,36 @@ test02()
 {
   int a[] = { 0, 1 };
   VERIFY( std::ranges::cdata(a) == a + 0 );
+
+  static_assert( has_cdata<int(&)[2]> );
+  static_assert( !has_cdata<int(&&)[2]> );
 }
 
-struct R
+struct R3
 {
-  long l = 0;
+  static inline int i = 0;
+  static inline long l = 0;
 
-  int* data() const { return nullptr; }
-  friend long* begin(R&& r); // this function is not defined
-  friend const long* begin(const R& r) { return &r.l; }
-  friend const short* begin(const R&&); // not defined
+  int* data() &; // this function is not defined
+  friend long* begin(R3&& r); // not defined
+  friend const long* begin(const R3& r) { return &r.l; }
+  friend const short* begin(const R3&&); // not defined
 };
 
-// This is a lie, ranges::begin(R&&) returns a dangling iterator.
-template<> constexpr bool std::ranges::enable_borrowed_range<R> = true;
+template<> constexpr bool std::ranges::enable_borrowed_range<R3> = true;
 
 void
 test03()
 {
-  R r;
-  const R& c = r;
+  static_assert( has_cdata<R3&> );
+  static_assert( has_cdata<R3> );  // borrowed range
+  static_assert( has_cdata<const R3&> );
+  static_assert( has_cdata<const R3> );  // borrowed range
+
+  R3 r;
+  const R3& c = r;
   VERIFY( std::ranges::cdata(r) == std::ranges::data(c) );
-  VERIFY( std::ranges::cdata(std::move(r)) == std::ranges::begin(c) );
+  VERIFY( std::ranges::cdata(std::move(r)) == std::ranges::data(c) );
   VERIFY( std::ranges::cdata(std::move(c)) == std::ranges::begin(c) );
 }
 
diff --git a/libstdc++-v3/testsuite/std/ranges/access/data.cc b/libstdc++-v3/testsuite/std/ranges/access/data.cc
index 40d3027a7df..237bbcc76c5 100644
--- a/libstdc++-v3/testsuite/std/ranges/access/data.cc
+++ b/libstdc++-v3/testsuite/std/ranges/access/data.cc
@@ -22,6 +22,10 @@
 #include <testsuite_hooks.h>
 #include <testsuite_iterators.h>
 
+template<typename T>
+  concept has_data
+    = requires (T&& t) { std::ranges::data(std::forward<T>(t)); };
+
 void
 test01()
 {
@@ -32,12 +36,18 @@ test01()
     int* data() { return &j; }
     const R* data() const noexcept { return nullptr; }
   };
+  static_assert( has_data<R&> );
+  static_assert( has_data<const R&> );
   R r;
   const R& c = r;
   VERIFY( std::ranges::data(r) == &r.j );
   static_assert( !noexcept(std::ranges::data(r)) );
   VERIFY( std::ranges::data(c) == (R*)nullptr );
   static_assert( noexcept(std::ranges::data(c)) );
+
+  // not lvalues and not borrowed ranges
+  static_assert( !has_data<R> );
+  static_assert( !has_data<const R> );
 }
 
 
@@ -49,31 +59,42 @@ test02()
 
   __gnu_test::test_range<int, __gnu_test::contiguous_iterator_wrapper> r(a);
   VERIFY( std::ranges::data(r) == std::to_address(std::ranges::begin(r)) );
+
+  static_assert( has_data<int(&)[2]> );
+  static_assert( has_data<decltype(r)&> );
+  static_assert( !has_data<int(&&)[2]> );
+  static_assert( !has_data<decltype(r)&&> );
 }
 
 struct R3
 {
-  long l = 0;
+  static inline int i;
+  static inline long l;
 
-  int* data() const { return nullptr; }
-  friend long* begin(R3& r) { return &r.l; }
-  friend const long* begin(const R3& r) { return &r.l + 1; }
+  int* data() & { return &i; }
+  friend long* begin(const R3& r) { return &l; }
+  friend const short* begin(const R3&&); // not defined
 };
 
-// N.B. this is a lie, begin on an R3 rvalue will return a dangling pointer.
 template<> constexpr bool std::ranges::enable_borrowed_range<R3> = true;
 
 void
 test03()
 {
+  static_assert( has_data<R3&> );
+  static_assert( has_data<R3> );  // borrowed range
+  static_assert( has_data<const R3&> );
+  static_assert( has_data<const R3> );  // borrowed range
+
   R3 r;
   const R3& c = r;
-  // r.data() can only be used on an lvalue, but ranges::begin(R3&&) is OK
-  // because R3 satisfies ranges::borrowed_range.
-  VERIFY( std::ranges::data(std::move(r)) == std::to_address(std::ranges::begin(std::move(r))) );
-  VERIFY( std::ranges::data(std::move(c)) == std::to_address(std::ranges::begin(std::move(c))) );
+  // PR libstdc++/100824
+  // ranges::data should treat the subexpression as an lvalue
+  VERIFY( std::ranges::data(std::move(r)) == &R3::i );
+  VERIFY( std::ranges::data(std::move(c)) == &R3::l );
 }
 
+
 int
 main()
 {
diff --git a/libstdc++-v3/testsuite/std/ranges/access/empty.cc b/libstdc++-v3/testsuite/std/ranges/access/empty.cc
index 1cddffa95a9..9044dd155da 100644
--- a/libstdc++-v3/testsuite/std/ranges/access/empty.cc
+++ b/libstdc++-v3/testsuite/std/ranges/access/empty.cc
@@ -35,7 +35,9 @@ test01()
   constexpr R r;
   static_assert( !std::ranges::empty(r) );
   static_assert( same_as<decltype(std::ranges::empty(r)), bool> );
-  static_assert( std::ranges::empty(std::move(r)) );
+  // PR libstdc++/100824
+  // ranges::empty should treat the subexpression as an lvalue
+  static_assert( !std::ranges::empty(std::move(r)) );
   static_assert( same_as<decltype(std::ranges::empty(std::move(r))), bool> );
 }
 
@@ -68,9 +70,29 @@ test02()
   VERIFY( !std::ranges::empty(so) );
 }
 
+void
+test03()
+{
+  // PR libstdc++/100824
+  // ranges::empty should treat the subexpression as an lvalue
+
+  struct R
+  {
+    constexpr bool empty() & { return true; }
+  };
+  static_assert( std::ranges::empty(R{}) );
+
+  struct R2
+  {
+    constexpr unsigned size() & { return 0; }
+  };
+  static_assert( std::ranges::empty(R2{}) );
+}
+
 int
 main()
 {
   test01();
   test02();
+  test03();
 }
diff --git a/libstdc++-v3/testsuite/std/ranges/access/size.cc b/libstdc++-v3/testsuite/std/ranges/access/size.cc
index 729308570ea..f25a1cb9ddb 100644
--- a/libstdc++-v3/testsuite/std/ranges/access/size.cc
+++ b/libstdc++-v3/testsuite/std/ranges/access/size.cc
@@ -76,12 +76,14 @@ test03()
   const R3& c = r;
   VERIFY( std::ranges::size(r) == 1 );
   static_assert( noexcept(std::ranges::size(r)) );
-  VERIFY( std::ranges::size(std::move(r)) == 3U );
-  static_assert( !noexcept(std::ranges::size(std::move(r))) );
+  // PR libstdc++/100824
+  // ranges::size should treat the subexpression as an lvalue
+  VERIFY( std::ranges::size(std::move(r)) == 1 );
+  static_assert( noexcept(std::ranges::size(std::move(r))) );
   VERIFY( std::ranges::size(c) == 2L );
   static_assert( !noexcept(std::ranges::size(c)) );
-  VERIFY( std::ranges::size(std::move(c)) == 4UL );
-  static_assert( noexcept(std::ranges::size(std::move(c))) );
+  VERIFY( std::ranges::size(std::move(c)) == 2L );
+  static_assert( !noexcept(std::ranges::size(std::move(c))) );
 }
 
 void
@@ -109,6 +111,15 @@ test05()
   VERIFY( std::ranges::size(r) == 1 );
 }
 
+void
+test06()
+{
+  // PR libstdc++/100824
+  // ranges::size should treat the subexpression as an lvalue
+  struct R { constexpr int size() & { return 42; } };
+  static_assert( std::ranges::size(R{}) == 42 );
+}
+
 int
 main()
 {
@@ -117,4 +128,5 @@ main()
   test03();
   test04();
   test05();
+  test06();
 }

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [committed] libstdc++: Fix value categories used by ranges access CPOs [PR 100824]
  2021-06-04 17:03 [committed] libstdc++: Fix value categories used by ranges access CPOs [PR 100824] Jonathan Wakely
@ 2021-06-04 20:44 ` Jonathan Wakely
  2021-06-04 20:46   ` Jonathan Wakely
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Wakely @ 2021-06-04 20:44 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2871 bytes --]

On 04/06/21 18:03 +0100, Jonathan Wakely wrote:
>The implementation of P2091R0 was incomplete, so that some range access
>CPOs used perfect forwarding where they should not. This fixes it by
>consistently operating on lvalues.
>
>Some additional changes that are not necessary to fix the bug:
>
>Modify the __as_const helper to simplify its usage. Instead of deducing
>the value category from its argument, and requiring callers to forward
>the argument as the correct category, add a non-deduced template
>parameter which is used for the value category and accept the argument
>as an lvalue. This means callers say __as_const<T>(t) instead of
>__as_const(std::forward<T>(t)).
>
>Always use an lvalue reference type as the template argument for the
>_S_noexcept helpers, so that we only instantiate one specialization for
>lvalues and rvalues of the same type.
>
>Move some helper concepts and functions from namespace std::__detail
>to ranges::__cust_access, to be consistent with the ranges::begin CPO.
>This ensures that the __adl_begin concept and the _Begin::operator()
>function are in the same namespace, so unqualified lookup is consistent
>and the poison pills for begin are visible to both.
>
>Simplified static assertions for arrays, because the expression a+0 is
>already ill-formed for an array of incomplete type.
>
>Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>
>libstdc++-v3/ChangeLog:
>
>	PR libstdc++/100824
>	* include/bits/iterator_concepts.h (__detail::__decay_copy)
>	(__detail::__member_begin, __detail::__adl_begin): Move to
>	namespace ranges::__cust_access.
>	(__detail::__ranges_begin): Likewise, and rename to __begin.
>	Remove redundant static assertion.
>	* include/bits/ranges_base.h (_Begin, _End, _RBegin, _REnd):
>	Use lvalue in noexcept specifier.
>	(__as_const): Add non-deduced parameter for value category.
>	(_CBegin, _CEnd, _CRBegin, _CREnd, _CData): Adjust uses of
>	__as_const.
>	(__member_size, __adl_size, __member_empty, __size0_empty):
>	(__eq_iter_empty, __adl_data): Use lvalue objects in
>	requirements.
>	(__sentinel_size): Likewise. Add check for conversion to
>	unsigned-like.
>	(__member_data): Allow non-lvalue types to satisfy the concept,
>	but use lvalue object in requirements.
>	(_Size, _SSize): Remove forwarding to always use an lvalue.
>	(_Data): Likewise. Add static assertion for arrays.
>	* testsuite/std/ranges/access/cdata.cc: Adjust expected
>	behaviour for rvalues. Add negative tests for ill-formed
>	expressions.
>	* testsuite/std/ranges/access/data.cc: Likewise.
>	* testsuite/std/ranges/access/empty.cc: Adjust expected
>	behaviour for rvalues.
>	* testsuite/std/ranges/access/size.cc: Likewise.

An additional problem with ranges::data was pointed out in the PR,
fixed with this patch.

Tested powerpc64le-linux. Committed to trunk.

This should also be backported to gcc-11 and gcc-10.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1846 bytes --]

commit 3e5f2425f80aedd00f28235022a2755eb46f310d
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 4 20:25:39 2021

    libstdc++: Fix helper concept for ranges::data [PR 100824]
    
    We need to decay the result of t.data() before checking if it's a
    pointer.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/100824
            * include/bits/ranges_base.h (__member_data): Use __decay_copy.
            * testsuite/std/ranges/access/data.cc: Add testcase from PR.

diff --git a/libstdc++-v3/include/bits/ranges_base.h b/libstdc++-v3/include/bits/ranges_base.h
index 17a421a4927..61d91eb8389 100644
--- a/libstdc++-v3/include/bits/ranges_base.h
+++ b/libstdc++-v3/include/bits/ranges_base.h
@@ -495,8 +495,10 @@ namespace ranges
 				    && is_object_v<remove_pointer_t<_Tp>>;
 
     template<typename _Tp>
-      concept __member_data
-	= requires(_Tp& __t) { { __t.data() } -> __pointer_to_object; };
+      concept __member_data = requires(_Tp& __t)
+	{
+	  { __cust_access::__decay_copy(__t.data()) } -> __pointer_to_object;
+	};
 
     template<typename _Tp>
       concept __begin_data = requires(_Tp& __t)
diff --git a/libstdc++-v3/testsuite/std/ranges/access/data.cc b/libstdc++-v3/testsuite/std/ranges/access/data.cc
index 237bbcc76c5..4f16f447f9f 100644
--- a/libstdc++-v3/testsuite/std/ranges/access/data.cc
+++ b/libstdc++-v3/testsuite/std/ranges/access/data.cc
@@ -92,8 +92,12 @@ test03()
   // ranges::data should treat the subexpression as an lvalue
   VERIFY( std::ranges::data(std::move(r)) == &R3::i );
   VERIFY( std::ranges::data(std::move(c)) == &R3::l );
-}
 
+  // PR libstdc++/100824 comment 3
+  // Check for member data() should use decay-copy
+  struct A { int*&& data(); };
+  static_assert( has_data<A&> );
+}
 
 int
 main()

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [committed] libstdc++: Fix value categories used by ranges access CPOs [PR 100824]
  2021-06-04 20:44 ` Jonathan Wakely
@ 2021-06-04 20:46   ` Jonathan Wakely
  2021-06-05 10:46     ` Jonathan Wakely
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Wakely @ 2021-06-04 20:46 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3147 bytes --]

On 04/06/21 21:44 +0100, Jonathan Wakely wrote:
>On 04/06/21 18:03 +0100, Jonathan Wakely wrote:
>>The implementation of P2091R0 was incomplete, so that some range access
>>CPOs used perfect forwarding where they should not. This fixes it by
>>consistently operating on lvalues.
>>
>>Some additional changes that are not necessary to fix the bug:
>>
>>Modify the __as_const helper to simplify its usage. Instead of deducing
>>the value category from its argument, and requiring callers to forward
>>the argument as the correct category, add a non-deduced template
>>parameter which is used for the value category and accept the argument
>>as an lvalue. This means callers say __as_const<T>(t) instead of
>>__as_const(std::forward<T>(t)).
>>
>>Always use an lvalue reference type as the template argument for the
>>_S_noexcept helpers, so that we only instantiate one specialization for
>>lvalues and rvalues of the same type.
>>
>>Move some helper concepts and functions from namespace std::__detail
>>to ranges::__cust_access, to be consistent with the ranges::begin CPO.
>>This ensures that the __adl_begin concept and the _Begin::operator()
>>function are in the same namespace, so unqualified lookup is consistent
>>and the poison pills for begin are visible to both.
>>
>>Simplified static assertions for arrays, because the expression a+0 is
>>already ill-formed for an array of incomplete type.
>>
>>Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>>
>>libstdc++-v3/ChangeLog:
>>
>>	PR libstdc++/100824
>>	* include/bits/iterator_concepts.h (__detail::__decay_copy)
>>	(__detail::__member_begin, __detail::__adl_begin): Move to
>>	namespace ranges::__cust_access.
>>	(__detail::__ranges_begin): Likewise, and rename to __begin.
>>	Remove redundant static assertion.
>>	* include/bits/ranges_base.h (_Begin, _End, _RBegin, _REnd):
>>	Use lvalue in noexcept specifier.
>>	(__as_const): Add non-deduced parameter for value category.
>>	(_CBegin, _CEnd, _CRBegin, _CREnd, _CData): Adjust uses of
>>	__as_const.
>>	(__member_size, __adl_size, __member_empty, __size0_empty):
>>	(__eq_iter_empty, __adl_data): Use lvalue objects in
>>	requirements.
>>	(__sentinel_size): Likewise. Add check for conversion to
>>	unsigned-like.
>>	(__member_data): Allow non-lvalue types to satisfy the concept,
>>	but use lvalue object in requirements.
>>	(_Size, _SSize): Remove forwarding to always use an lvalue.
>>	(_Data): Likewise. Add static assertion for arrays.
>>	* testsuite/std/ranges/access/cdata.cc: Adjust expected
>>	behaviour for rvalues. Add negative tests for ill-formed
>>	expressions.
>>	* testsuite/std/ranges/access/data.cc: Likewise.
>>	* testsuite/std/ranges/access/empty.cc: Adjust expected
>>	behaviour for rvalues.
>>	* testsuite/std/ranges/access/size.cc: Likewise.
>
>An additional problem with ranges::data was pointed out in the PR,
>fixed with this patch.

And this implements the rest of LWG 3403. The change to the
ranges::ssize constraints was already done by the first patch in this
thread, this fixes the return type.

Tested powerpc64le-linux. Committed to trunk.

This should also be backported to gcc-11 and gcc-10.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 3446 bytes --]

commit 621ea10ca060ba19ec693aa73b5e29d553cca849
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 4 20:28:04 2021

    libstdc++: Implement LWG 3403 for std::ranges::ssize
    
    I already changed the constraints for ranges::ssize to use ranges::size,
    this implements the rest of LWG 3403, so that the returned type is the
    signed type corresponding to the result of ranges::size.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/ranges_base.h (_SSize): Return the result of
            ranges::size converted to the wider of make-signed-like-t<S> and
            ptrdiff_t, rather than the ranges different type.
            * testsuite/std/ranges/access/ssize.cc: Adjust expected result
            for an iota_view that uses an integer class type for its
            difference_type.

diff --git a/libstdc++-v3/include/bits/ranges_base.h b/libstdc++-v3/include/bits/ranges_base.h
index 61d91eb8389..e3c3962bcd9 100644
--- a/libstdc++-v3/include/bits/ranges_base.h
+++ b/libstdc++-v3/include/bits/ranges_base.h
@@ -425,22 +425,32 @@ namespace ranges
 
     struct _SSize
     {
+      // _GLIBCXX_RESOLVE_LIB_DEFECTS
+      // 3403. Domain of ranges::ssize(E) doesn't match ranges::size(E)
       template<typename _Tp>
 	requires requires (_Tp& __t) { _Size{}(__t); }
 	constexpr auto
 	operator()(_Tp&& __t) const noexcept(noexcept(_Size{}(__t)))
 	{
-	  using __iter_type = decltype(_Begin{}(__t));
-	  using __diff_type = iter_difference_t<__iter_type>;
-	  using __gnu_cxx::__int_traits;
 	  auto __size = _Size{}(__t);
-	  if constexpr (integral<__diff_type>)
+	  using __size_type = decltype(__size);
+	  // Return the wider of ptrdiff_t and make-signed-like-t<__size_type>.
+	  if constexpr (integral<__size_type>)
 	    {
-	      if constexpr (__int_traits<__diff_type>::__digits
+	      using __gnu_cxx::__int_traits;
+	      if constexpr (__int_traits<__size_type>::__digits
 			    < __int_traits<ptrdiff_t>::__digits)
 		return static_cast<ptrdiff_t>(__size);
+	      else
+		return static_cast<make_signed_t<__size_type>>(__size);
 	    }
-	  return static_cast<__diff_type>(__size);
+#if defined __STRICT_ANSI__ && defined __SIZEOF_INT128__
+	  // For strict-ansi modes integral<__int128> is false
+	  else if constexpr (__detail::__is_int128<__size_type>)
+	    return static_cast<unsigned __int128>(__size);
+#endif
+	  else // Must be one of __max_diff_type or __max_size_type.
+	    return __detail::__max_diff_type(__size);
 	}
     };
 
diff --git a/libstdc++-v3/testsuite/std/ranges/access/ssize.cc b/libstdc++-v3/testsuite/std/ranges/access/ssize.cc
index b00ec5db1f8..f53f462580c 100644
--- a/libstdc++-v3/testsuite/std/ranges/access/ssize.cc
+++ b/libstdc++-v3/testsuite/std/ranges/access/ssize.cc
@@ -76,9 +76,12 @@ void
 test06()
 {
   auto i = std::views::iota(1ull, 5u);
-  auto s = std::ranges::ssize(i);
-  using R = std::ranges::range_difference_t<decltype(i)>;
-  static_assert( std::same_as<decltype(s), R> );
+  auto s = std::ranges::size(i);
+  auto ss = std::ranges::ssize(i);
+  // std::ranges::range_difference_t<decltype(i)> is larger than long long,
+  // but LWG 3403 says ranges::ssize(i) returns the signed version of the
+  // type that ranges::size(i) returns, not the range's difference_type.
+  static_assert( std::same_as<decltype(ss), std::make_signed_t<decltype(s)>> );
   VERIFY( s == 4 );
 }
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [committed] libstdc++: Fix value categories used by ranges access CPOs [PR 100824]
  2021-06-04 20:46   ` Jonathan Wakely
@ 2021-06-05 10:46     ` Jonathan Wakely
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2021-06-05 10:46 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3314 bytes --]

On 04/06/21 21:46 +0100, Jonathan Wakely wrote:
>On 04/06/21 21:44 +0100, Jonathan Wakely wrote:
>>On 04/06/21 18:03 +0100, Jonathan Wakely wrote:
>>>The implementation of P2091R0 was incomplete, so that some range access
>>>CPOs used perfect forwarding where they should not. This fixes it by
>>>consistently operating on lvalues.
>>>
>>>Some additional changes that are not necessary to fix the bug:
>>>
>>>Modify the __as_const helper to simplify its usage. Instead of deducing
>>>the value category from its argument, and requiring callers to forward
>>>the argument as the correct category, add a non-deduced template
>>>parameter which is used for the value category and accept the argument
>>>as an lvalue. This means callers say __as_const<T>(t) instead of
>>>__as_const(std::forward<T>(t)).
>>>
>>>Always use an lvalue reference type as the template argument for the
>>>_S_noexcept helpers, so that we only instantiate one specialization for
>>>lvalues and rvalues of the same type.
>>>
>>>Move some helper concepts and functions from namespace std::__detail
>>>to ranges::__cust_access, to be consistent with the ranges::begin CPO.
>>>This ensures that the __adl_begin concept and the _Begin::operator()
>>>function are in the same namespace, so unqualified lookup is consistent
>>>and the poison pills for begin are visible to both.
>>>
>>>Simplified static assertions for arrays, because the expression a+0 is
>>>already ill-formed for an array of incomplete type.
>>>
>>>Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>>>
>>>libstdc++-v3/ChangeLog:
>>>
>>>	PR libstdc++/100824
>>>	* include/bits/iterator_concepts.h (__detail::__decay_copy)
>>>	(__detail::__member_begin, __detail::__adl_begin): Move to
>>>	namespace ranges::__cust_access.
>>>	(__detail::__ranges_begin): Likewise, and rename to __begin.
>>>	Remove redundant static assertion.
>>>	* include/bits/ranges_base.h (_Begin, _End, _RBegin, _REnd):
>>>	Use lvalue in noexcept specifier.
>>>	(__as_const): Add non-deduced parameter for value category.
>>>	(_CBegin, _CEnd, _CRBegin, _CREnd, _CData): Adjust uses of
>>>	__as_const.
>>>	(__member_size, __adl_size, __member_empty, __size0_empty):
>>>	(__eq_iter_empty, __adl_data): Use lvalue objects in
>>>	requirements.
>>>	(__sentinel_size): Likewise. Add check for conversion to
>>>	unsigned-like.
>>>	(__member_data): Allow non-lvalue types to satisfy the concept,
>>>	but use lvalue object in requirements.
>>>	(_Size, _SSize): Remove forwarding to always use an lvalue.
>>>	(_Data): Likewise. Add static assertion for arrays.
>>>	* testsuite/std/ranges/access/cdata.cc: Adjust expected
>>>	behaviour for rvalues. Add negative tests for ill-formed
>>>	expressions.
>>>	* testsuite/std/ranges/access/data.cc: Likewise.
>>>	* testsuite/std/ranges/access/empty.cc: Adjust expected
>>>	behaviour for rvalues.
>>>	* testsuite/std/ranges/access/size.cc: Likewise.
>>
>>An additional problem with ranges::data was pointed out in the PR,
>>fixed with this patch.
>
>And this implements the rest of LWG 3403. The change to the
>ranges::ssize constraints was already done by the first patch in this
>thread, this fixes the return type.

Aaaaand one more fix, as pointed out in the PR.

Tested x86_64-linux. Committed to trunk.

And this one should also be backported to gcc-11 and gcc-10.




[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1807 bytes --]

commit 96963713f6a648a0ed890450e02ebdd8ff583b14
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sat Jun 5 11:42:01 2021

    libstdc++: Fix return type of ranges::ssize for 128-bit integer [PR 100824]
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/100824
            * include/bits/ranges_base.h (_SSize): Return signed type.
            * testsuite/std/ranges/access/ssize.cc: Check with __int128.

diff --git a/libstdc++-v3/include/bits/ranges_base.h b/libstdc++-v3/include/bits/ranges_base.h
index e3c3962bcd9..728d3ad2b25 100644
--- a/libstdc++-v3/include/bits/ranges_base.h
+++ b/libstdc++-v3/include/bits/ranges_base.h
@@ -447,7 +447,7 @@ namespace ranges
 #if defined __STRICT_ANSI__ && defined __SIZEOF_INT128__
 	  // For strict-ansi modes integral<__int128> is false
 	  else if constexpr (__detail::__is_int128<__size_type>)
-	    return static_cast<unsigned __int128>(__size);
+	    return static_cast<__int128>(__size);
 #endif
 	  else // Must be one of __max_diff_type or __max_size_type.
 	    return __detail::__max_diff_type(__size);
diff --git a/libstdc++-v3/testsuite/std/ranges/access/ssize.cc b/libstdc++-v3/testsuite/std/ranges/access/ssize.cc
index f53f462580c..fdbf245d036 100644
--- a/libstdc++-v3/testsuite/std/ranges/access/ssize.cc
+++ b/libstdc++-v3/testsuite/std/ranges/access/ssize.cc
@@ -85,6 +85,20 @@ test06()
   VERIFY( s == 4 );
 }
 
+void
+test07()
+{
+#ifdef __SIZEOF_INT128__
+  struct R
+  {
+    unsigned __int128 size() const { return 4; }
+  };
+  R r;
+  static_assert( std::same_as<decltype(std::ranges::ssize(r)), __int128> );
+  VERIFY( std::ranges::ssize(r) == 4 );
+#endif
+}
+
 int
 main()
 {
@@ -93,4 +107,5 @@ main()
   test04();
   test05();
   test06();
+  test07();
 }

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-05 10:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-04 17:03 [committed] libstdc++: Fix value categories used by ranges access CPOs [PR 100824] Jonathan Wakely
2021-06-04 20:44 ` Jonathan Wakely
2021-06-04 20:46   ` Jonathan Wakely
2021-06-05 10:46     ` Jonathan Wakely

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).