public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix handling of arrays in range access customization points
@ 2019-10-31 13:17 Jonathan Wakely
  0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2019-10-31 13:17 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

This combines the overloads for arrays with the non-array overloads,
using if-constexpr to choose between the cases, and also enforces the
following:

- ADL should only be used for class types and enumeration types.
- ranges::begin should be ill-formed for rvalue arrays.
- ranges::end should be ill-formed for rvalue arrays, unbounded
  arrays, and arrays of incomplete type.
- ranges::size should be ill-formed for unbounded arrays.

	* include/bits/range_access.h (ranges::begin): Combine array and
	non-array overloads into one function template. Only use ADL for
	classes and enums
	(ranges::end, ranges::size): Likewise. Make unbounded arrays
	ill-formed.
	(ranges::rbegin, ranges::rend): Only use ADL for classes and enums.
	Reformat _S_noexcept() functions to mirror operator() structure.
	* testsuite/std/ranges/access/begin.cc: Check incomplete array.
	* testsuite/std/ranges/access/end_neg.cc: New test.
	* testsuite/std/ranges/access/size.cc: Check array of incomplete type.
	* testsuite/std/ranges/access/size_neg.cc: New test.

This implements what the standard *should* say, not what's in the
current draft. I submitted a ballot comment about the unbounded arrays
part and have been talking to Casey Carter about cleaning up the spec
to match what this patch implements.

Tested powerpc64le-linux, committed to trunk.


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

commit 83314d59a0072767a1945cebaf28c626184b98c2
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Oct 30 17:49:47 2019 +0000

    Fix handling of arrays in range access customization points
    
    This combines the overloads for arrays with the non-array overloads,
    using if-constexpr to choose between the cases, and also enforces the
    following:
    
    - ADL should only be used for class types and enumeration types.
    - ranges::begin should be ill-formed for rvalue arrays.
    - ranges::end should be ill-formed for rvalue arrays, unbounded
      arrays, and arrays of incomplete type.
    - ranges::size should be ill-formed for unbounded arrays.
    
            * include/bits/range_access.h (ranges::begin): Combine array and
            non-array overloads into one function template. Only use ADL for
            classes and enums
            (ranges::end, ranges::size): Likewise. Make unbounded arrays
            ill-formed.
            (ranges::rbegin, ranges::rend): Only use ADL for classes and enums.
            Reformat _S_noexcept() functions to mirror operator() structure.
            * testsuite/std/ranges/access/begin.cc: Check incomplete array.
            * testsuite/std/ranges/access/end_neg.cc: New test.
            * testsuite/std/ranges/access/size.cc: Check array of incomplete type.
            * testsuite/std/ranges/access/size_neg.cc: New test.

diff --git a/libstdc++-v3/include/bits/range_access.h b/libstdc++-v3/include/bits/range_access.h
index 9c8bef6b3a1..f0a339277e9 100644
--- a/libstdc++-v3/include/bits/range_access.h
+++ b/libstdc++-v3/include/bits/range_access.h
@@ -391,15 +391,14 @@ namespace ranges
     template<typename _Tp> void begin(initializer_list<_Tp>&&) = delete;
 
     template<typename _Tp>
-      concept __adl_begin = requires(_Tp&& __t)
+      concept __adl_begin
+	= std::__detail::__class_or_enum<remove_reference_t<_Tp>>
+	&& requires(_Tp&& __t)
 	{
 	  { __decay_copy(begin(std::forward<_Tp>(__t))) }
 	    -> input_or_output_iterator;
 	};
 
-    template<typename _Tp>
-      concept __complete_type = requires(_Tp* __p) { __p + 1; };
-
     struct _Begin
     {
     private:
@@ -407,23 +406,27 @@ namespace ranges
 	static constexpr bool
 	_S_noexcept()
 	{
-	  if constexpr (__member_begin<_Tp>)
+	  if constexpr (is_array_v<remove_reference_t<_Tp>>)
+	    return true;
+	  else if constexpr (__member_begin<_Tp>)
 	    return noexcept(__decay_copy(std::declval<_Tp>().begin()));
 	  else
 	    return noexcept(__decay_copy(begin(std::declval<_Tp>())));
 	}
 
     public:
-      template<__complete_type _Tp, size_t _Nm>
-	constexpr _Tp*
-	operator()(_Tp (&__e)[_Nm]) const noexcept
-	{ return __e; }
-
-      template<typename _Tp> requires __member_begin<_Tp> || __adl_begin<_Tp>
+      template<typename _Tp>
+	requires is_array_v<remove_reference_t<_Tp>> || __member_begin<_Tp>
+	  || __adl_begin<_Tp>
 	constexpr auto
 	operator()(_Tp&& __e) const noexcept(_S_noexcept<_Tp>())
 	{
-	  if constexpr (__member_begin<_Tp>)
+	  if constexpr (is_array_v<remove_reference_t<_Tp>>)
+	    {
+	      static_assert(is_lvalue_reference_v<_Tp>);
+	      return __e;
+	    }
+	  else if constexpr (__member_begin<_Tp>)
 	    return __e.begin();
 	  else
 	    return begin(std::forward<_Tp>(__e));
@@ -442,7 +445,9 @@ namespace ranges
     template<typename _Tp> void end(initializer_list<_Tp>&&) = delete;
 
     template<typename _Tp>
-      concept __adl_end = requires(_Tp&& __t)
+      concept __adl_end
+	= std::__detail::__class_or_enum<remove_reference_t<_Tp>>
+	&& requires(_Tp&& __t)
 	{
 	  { __decay_copy(end(std::forward<_Tp>(__t))) }
 	    -> sentinel_for<decltype(_Begin{}(std::forward<_Tp>(__t)))>;
@@ -455,23 +460,28 @@ namespace ranges
 	static constexpr bool
 	_S_noexcept()
 	{
-	  if constexpr (__member_end<_Tp>)
+	  if constexpr (is_array_v<remove_reference_t<_Tp>>)
+	    return true;
+	  else if constexpr (__member_end<_Tp>)
 	    return noexcept(__decay_copy(std::declval<_Tp>().end()));
 	  else
 	    return noexcept(__decay_copy(end(std::declval<_Tp>())));
 	}
 
     public:
-      template<__complete_type _Tp, size_t _Nm>
-	constexpr _Tp*
-	operator()(_Tp (&__e)[_Nm]) const noexcept
-	{ return __e + _Nm; }
-
-      template<typename _Tp> requires __member_end<_Tp> || __adl_end<_Tp>
+      template<typename _Tp>
+	requires is_array_v<remove_reference_t<_Tp>> || __member_end<_Tp>
+	|| __adl_end<_Tp>
 	constexpr auto
 	operator()(_Tp&& __e) const noexcept(_S_noexcept<_Tp>())
 	{
-	  if constexpr (__member_end<_Tp>)
+	  if constexpr (is_array_v<remove_reference_t<_Tp>>)
+	    {
+	      static_assert(is_lvalue_reference_v<_Tp>);
+	      static_assert(is_bounded_array_v<remove_reference_t<_Tp>>);
+	      return __e + extent_v<remove_reference_t<_Tp>>;
+	    }
+	  else if constexpr (__member_end<_Tp>)
 	    return __e.end();
 	  else
 	    return end(std::forward<_Tp>(__e));
@@ -520,7 +530,9 @@ namespace ranges
     template<typename _Tp> void rbegin(_Tp&&) = delete;
 
     template<typename _Tp>
-      concept __adl_rbegin = requires(_Tp&& __t)
+      concept __adl_rbegin
+	= std::__detail::__class_or_enum<remove_reference_t<_Tp>>
+	&& requires(_Tp&& __t)
 	{
 	  { __decay_copy(rbegin(std::forward<_Tp>(__t))) }
 	    -> input_or_output_iterator;
@@ -545,14 +557,17 @@ namespace ranges
 	    return noexcept(__decay_copy(std::declval<_Tp>().rbegin()));
 	  else if constexpr (__adl_rbegin<_Tp>)
 	    return noexcept(__decay_copy(rbegin(std::declval<_Tp>())));
-	  else if constexpr (noexcept(_End{}(std::declval<_Tp>())))
-	    {
-	      using _It = decltype(_End{}(std::declval<_Tp>()));
-	      // std::reverse_iterator copy-initializes its member.
-	      return is_nothrow_copy_constructible_v<_It>;
-	    }
 	  else
-	    return false;
+	    {
+	      if constexpr (noexcept(_End{}(std::declval<_Tp>())))
+		{
+		  using _It = decltype(_End{}(std::declval<_Tp>()));
+		  // std::reverse_iterator copy-initializes its member.
+		  return is_nothrow_copy_constructible_v<_It>;
+		}
+	      else
+		return false;
+	    }
 	}
 
     public:
@@ -582,7 +597,9 @@ namespace ranges
     template<typename _Tp> void rend(_Tp&&) = delete;
 
     template<typename _Tp>
-      concept __adl_rend = requires(_Tp&& __t)
+      concept __adl_rend
+	= std::__detail::__class_or_enum<remove_reference_t<_Tp>>
+	&& requires(_Tp&& __t)
 	{
 	  { __decay_copy(rend(std::forward<_Tp>(__t))) }
 	    -> sentinel_for<decltype(_RBegin{}(std::forward<_Tp>(__t)))>;
@@ -599,14 +616,17 @@ namespace ranges
 	    return noexcept(__decay_copy(std::declval<_Tp>().rend()));
 	  else if constexpr (__adl_rend<_Tp>)
 	    return noexcept(__decay_copy(rend(std::declval<_Tp>())));
-	  else if constexpr (noexcept(_Begin{}(std::declval<_Tp>())))
-	    {
-	      using _It = decltype(_Begin{}(std::declval<_Tp>()));
-	      // std::reverse_iterator copy-initializes its member.
-	      return is_nothrow_copy_constructible_v<_It>;
-	    }
 	  else
-	    return false;
+	    {
+	      if constexpr (noexcept(_Begin{}(std::declval<_Tp>())))
+		{
+		  using _It = decltype(_Begin{}(std::declval<_Tp>()));
+		  // std::reverse_iterator copy-initializes its member.
+		  return is_nothrow_copy_constructible_v<_It>;
+		}
+	      else
+		return false;
+	    }
 	}
 
     public:
@@ -660,7 +680,9 @@ namespace ranges
     template<typename _Tp> void size(_Tp&&) = delete;
 
     template<typename _Tp>
-      concept __adl_size = !disable_sized_range<remove_cvref_t<_Tp>>
+      concept __adl_size
+	= std::__detail::__class_or_enum<remove_reference_t<_Tp>>
+	&& !disable_sized_range<remove_cvref_t<_Tp>>
 	&& requires(_Tp&& __t)
 	{
 	  { __decay_copy(size(std::forward<_Tp>(__t))) }
@@ -689,7 +711,9 @@ namespace ranges
 	static constexpr bool
 	_S_noexcept()
 	{
-	  if constexpr (__member_size<_Tp>)
+	  if constexpr (is_array_v<remove_reference_t<_Tp>>)
+	    return true;
+	  else if constexpr (__member_size<_Tp>)
 	    return noexcept(__decay_copy(std::declval<_Tp>().size()));
 	  else if constexpr (__adl_size<_Tp>)
 	    return noexcept(__decay_copy(size(std::declval<_Tp>())));
@@ -699,17 +723,18 @@ namespace ranges
 	}
 
     public:
-      template<__complete_type _Tp, size_t _Nm>
-	constexpr size_t
-	operator()(_Tp (&__e)[_Nm]) const noexcept
-	{ return _Nm; }
-
       template<typename _Tp>
-	requires __member_size<_Tp> || __adl_size<_Tp> || __sizable<_Tp>
+	requires is_array_v<remove_reference_t<_Tp>>
+	  || __member_size<_Tp> || __adl_size<_Tp> || __sizable<_Tp>
 	constexpr auto
 	operator()(_Tp&& __e) const noexcept(_S_noexcept<_Tp>())
 	{
-	  if constexpr (__member_size<_Tp>)
+	  if constexpr (is_array_v<remove_reference_t<_Tp>>)
+	    {
+	      static_assert(is_bounded_array_v<remove_reference_t<_Tp>>);
+	      return extent_v<remove_reference_t<_Tp>>;
+	    }
+	  else if constexpr (__member_size<_Tp>)
 	    return std::forward<_Tp>(__e).size();
 	  else if constexpr (__adl_size<_Tp>)
 	    return size(std::forward<_Tp>(__e));
diff --git a/libstdc++-v3/testsuite/std/ranges/access/begin.cc b/libstdc++-v3/testsuite/std/ranges/access/begin.cc
index 1d7db46b68a..100dcf69c6e 100644
--- a/libstdc++-v3/testsuite/std/ranges/access/begin.cc
+++ b/libstdc++-v3/testsuite/std/ranges/access/begin.cc
@@ -35,6 +35,11 @@ test01()
 
   constexpr long b[2] = { };
   static_assert( std::ranges::begin(b) == (b + 0) );
+
+  struct Incomplete;
+  using A = Incomplete[]; // unbounded array of incomplete type
+  extern A& f();
+  static_assert( same_as<decltype(std::ranges::begin(f())), Incomplete*> );
 }
 
 void
diff --git a/libstdc++-v3/testsuite/std/ranges/access/end_neg.cc b/libstdc++-v3/testsuite/std/ranges/access/end_neg.cc
new file mode 100644
index 00000000000..a2a8fb05f92
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/access/end_neg.cc
@@ -0,0 +1,42 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <iterator> // N.B. should be <ranges>
+
+extern int unbounded[];
+
+auto
+test01()
+{
+  return std::ranges::end(unbounded); // { dg-error "here" }
+}
+// { dg-error "static assertion failed" "" { target *-*-* } 0 }
+
+struct incomplete;
+extern incomplete array[2];
+
+auto
+test02()
+{
+  return std::ranges::end(array); // { dg-error "here" }
+}
+// { dg-error "incomplete type" "" { target *-*-* } 0 }
+
+
diff --git a/libstdc++-v3/testsuite/std/ranges/access/size.cc b/libstdc++-v3/testsuite/std/ranges/access/size.cc
index b92e5d50449..b0a27ca2a87 100644
--- a/libstdc++-v3/testsuite/std/ranges/access/size.cc
+++ b/libstdc++-v3/testsuite/std/ranges/access/size.cc
@@ -32,6 +32,11 @@ test01()
   int a2[2];
   VERIFY( std::ranges::size(a2) == 2);
   static_assert( noexcept(std::ranges::size(a2)) );
+
+  struct Incomplete;
+  using A = Incomplete[2]; // bounded array of incomplete type
+  extern A& f();
+  static_assert( std::same_as<decltype(std::ranges::size(f())), std::size_t> );
 }
 
 void
@@ -84,8 +89,7 @@ test04()
 {
   int a[] = { 0, 1 };
   __gnu_test::test_range<int, __gnu_test::random_access_iterator_wrapper> r(a);
-  auto& rr = r;
-  VERIFY( std::ranges::size(r) == (std::ranges::end(r) - std::ranges::begin(r)) );
+  VERIFY( std::ranges::size(r) == unsigned(std::ranges::end(r) - std::ranges::begin(r)) );
 }
 
 struct R5
diff --git a/libstdc++-v3/testsuite/std/ranges/access/size_neg.cc b/libstdc++-v3/testsuite/std/ranges/access/size_neg.cc
new file mode 100644
index 00000000000..0ba8d81874f
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/access/size_neg.cc
@@ -0,0 +1,30 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <iterator> // N.B. should be <ranges>
+
+extern int unbounded[];
+
+auto
+test01()
+{
+  return std::ranges::size(unbounded); // { dg-error "here" }
+}
+// { dg-error "static assertion failed" "" { target *-*-* } 0 }

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

only message in thread, other threads:[~2019-10-31 13:17 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-31 13:17 [PATCH] Fix handling of arrays in range access customization points 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).