[committed] libstdc++: Fix std::get<T> for std::tuple [PR101427]

[committed] libstdc++: Fix std::get<T> for std::tuple [PR101427]

[Jonathan Wakely]

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

The std::get<T> functions relied on deduction failing if more than one
base class existed for the type T.  However the implementation of Core
DR 2303 (in r11-4693) made deduction succeed (and select the
more-derived base class).

This rewrites the implementation of std::get<T> to explicitly check for
more than one occurrence of T in the tuple elements, making it
ill-formed again. Additionally, the large wall of overload resolution
errors described in PR c++/101460 is avoided by making std::get<T> use
__get_helper<I> directly instead of calling std::get<I>, and by adding a
deleted overload of __get_helper<N> for out-of-range N.

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

libstdc++-v3/ChangeLog:

	PR libstdc++/101427
	* include/std/tuple (tuple_element): Improve static_assert text.
	(__get_helper): Add deleted overload.
	(get<i>(tuple<T...>&&), get<i>(const tuple<T...>&&)): Use
	__get_helper directly.
	(__get_helper2): Remove.
	(__find_uniq_type_in_pack): New constexpr helper function.
	(get<T>): Use __find_uniq_type_in_pack and __get_helper instead
	of __get_helper2.
	* testsuite/20_util/tuple/element_access/get_neg.cc: Adjust
	expected errors.
	* testsuite/20_util/tuple/element_access/101427.cc: New test.

Tested powerpc64le-linux. Committed to trunk.


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

commit 17855eed7fc76b2cee7fbbc26f84d3c8b99be13c
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jul 14 20:14:14 2021

    libstdc++: Fix std::get<T> for std::tuple [PR101427]
    
    The std::get<T> functions relied on deduction failing if more than one
    base class existed for the type T.  However the implementation of Core
    DR 2303 (in r11-4693) made deduction succeed (and select the
    more-derived base class).
    
    This rewrites the implementation of std::get<T> to explicitly check for
    more than one occurrence of T in the tuple elements, making it
    ill-formed again. Additionally, the large wall of overload resolution
    errors described in PR c++/101460 is avoided by making std::get<T> use
    __get_helper<I> directly instead of calling std::get<I>, and by adding a
    deleted overload of __get_helper<N> for out-of-range N.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/101427
            * include/std/tuple (tuple_element): Improve static_assert text.
            (__get_helper): Add deleted overload.
            (get<i>(tuple<T...>&&), get<i>(const tuple<T...>&&)): Use
            __get_helper directly.
            (__get_helper2): Remove.
            (__find_uniq_type_in_pack): New constexpr helper function.
            (get<T>): Use __find_uniq_type_in_pack and __get_helper instead
            of __get_helper2.
            * testsuite/20_util/tuple/element_access/get_neg.cc: Adjust
            expected errors.
            * testsuite/20_util/tuple/element_access/101427.cc: New test.

diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 2d562f8da77..6953f8715d7 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -1358,7 +1358,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct tuple_element<__i, tuple<>>
     {
       static_assert(__i < tuple_size<tuple<>>::value,
-	  "tuple index is in range");
+	  "tuple index must be in range");
     };
 
   template<size_t __i, typename _Head, typename... _Tail>
@@ -1371,6 +1371,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     __get_helper(const _Tuple_impl<__i, _Head, _Tail...>& __t) noexcept
     { return _Tuple_impl<__i, _Head, _Tail...>::_M_head(__t); }
 
+  // Deleted overload to improve diagnostics for invalid indices
+  template<size_t __i, typename... _Types>
+    __enable_if_t<(__i >= sizeof...(_Types))>
+    __get_helper(const tuple<_Types...>&) = delete;
+
   /// Return a reference to the ith element of a tuple.
   template<size_t __i, typename... _Elements>
     constexpr __tuple_element_t<__i, tuple<_Elements...>>&
@@ -1389,7 +1394,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     get(tuple<_Elements...>&& __t) noexcept
     {
       typedef __tuple_element_t<__i, tuple<_Elements...>> __element_type;
-      return std::forward<__element_type&&>(std::get<__i>(__t));
+      return std::forward<__element_type>(std::__get_helper<__i>(__t));
     }
 
   /// Return a const rvalue reference to the ith element of a const tuple rvalue.
@@ -1398,47 +1403,79 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     get(const tuple<_Elements...>&& __t) noexcept
     {
       typedef __tuple_element_t<__i, tuple<_Elements...>> __element_type;
-      return std::forward<const __element_type&&>(std::get<__i>(__t));
+      return std::forward<const __element_type>(std::__get_helper<__i>(__t));
     }
 
 #if __cplusplus >= 201402L
 
 #define __cpp_lib_tuples_by_type 201304
 
-  template<typename _Head, size_t __i, typename... _Tail>
-    constexpr _Head&
-    __get_helper2(_Tuple_impl<__i, _Head, _Tail...>& __t) noexcept
-    { return _Tuple_impl<__i, _Head, _Tail...>::_M_head(__t); }
-
-  template<typename _Head, size_t __i, typename... _Tail>
-    constexpr const _Head&
-    __get_helper2(const _Tuple_impl<__i, _Head, _Tail...>& __t) noexcept
-    { return _Tuple_impl<__i, _Head, _Tail...>::_M_head(__t); }
+  // Return the index of _Tp in _Types, if it occurs exactly once.
+  // Otherwise, return sizeof...(_Types).
+  // TODO reuse this for __detail::__variant::__exactly_once.
+  template<typename _Tp, typename... _Types>
+    constexpr size_t
+    __find_uniq_type_in_pack()
+    {
+      constexpr size_t __sz = sizeof...(_Types);
+      constexpr bool __found[__sz] = { __is_same(_Tp, _Types) ... };
+      size_t __n = __sz;
+      for (size_t __i = 0; __i < __sz; ++__i)
+	{
+	  if (__found[__i])
+	    {
+	      if (__n < __sz) // more than one _Tp found
+		return __sz;
+	      __n = __i;
+	    }
+	}
+      return __n;
+    }
 
   /// Return a reference to the unique element of type _Tp of a tuple.
   template <typename _Tp, typename... _Types>
     constexpr _Tp&
     get(tuple<_Types...>& __t) noexcept
-    { return std::__get_helper2<_Tp>(__t); }
+    {
+      constexpr size_t __idx = __find_uniq_type_in_pack<_Tp, _Types...>();
+      static_assert(__idx < sizeof...(_Types),
+	  "the type T in std::get<T> must occur exactly once in the tuple");
+      return std::__get_helper<__idx>(__t);
+    }
 
   /// Return a reference to the unique element of type _Tp of a tuple rvalue.
   template <typename _Tp, typename... _Types>
     constexpr _Tp&&
     get(tuple<_Types...>&& __t) noexcept
-    { return std::forward<_Tp&&>(std::__get_helper2<_Tp>(__t)); }
+    {
+      constexpr size_t __idx = __find_uniq_type_in_pack<_Tp, _Types...>();
+      static_assert(__idx < sizeof...(_Types),
+	  "the type T in std::get<T> must occur exactly once in the tuple");
+      return std::forward<_Tp>(std::__get_helper<__idx>(__t));
+    }
 
   /// Return a const reference to the unique element of type _Tp of a tuple.
   template <typename _Tp, typename... _Types>
     constexpr const _Tp&
     get(const tuple<_Types...>& __t) noexcept
-    { return std::__get_helper2<_Tp>(__t); }
+    {
+      constexpr size_t __idx = __find_uniq_type_in_pack<_Tp, _Types...>();
+      static_assert(__idx < sizeof...(_Types),
+	  "the type T in std::get<T> must occur exactly once in the tuple");
+      return std::__get_helper<__idx>(__t);
+    }
 
   /// Return a const reference to the unique element of type _Tp of
   /// a const tuple rvalue.
   template <typename _Tp, typename... _Types>
     constexpr const _Tp&&
     get(const tuple<_Types...>&& __t) noexcept
-    { return std::forward<const _Tp&&>(std::__get_helper2<_Tp>(__t)); }
+    {
+      constexpr size_t __idx = __find_uniq_type_in_pack<_Tp, _Types...>();
+      static_assert(__idx < sizeof...(_Types),
+	  "the type T in std::get<T> must occur exactly once in the tuple");
+      return std::forward<const _Tp>(std::__get_helper<__idx>(__t));
+    }
 #endif
 
   // This class performs the comparison operations on tuples
diff --git a/libstdc++-v3/testsuite/20_util/tuple/element_access/101427.cc b/libstdc++-v3/testsuite/20_util/tuple/element_access/101427.cc
new file mode 100644
index 00000000000..6490afda193
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/tuple/element_access/101427.cc
@@ -0,0 +1,23 @@
+// { dg-do compile { target c++14 } }
+// PR libstdc++/101427
+
+#include <tuple>
+
+void test_pr101427()
+{
+  std::tuple<int, int> tup1;
+  std::get<int>(tup1); // { dg-error "here" }
+
+  const std::tuple<int, long, int, long> tup2;
+  std::get<long>(tup2); // { dg-error "here" }
+
+  std::tuple<char, short, float, short, int> tup3;
+  std::get<short>(std::move(tup3)); // { dg-error "here" }
+
+  const std::tuple<double, long, double, long> tup4;
+  std::get<double>(std::move(tup4)); // { dg-error "here" }
+
+  // { dg-error "must occur exactly once in the tuple" "" { target *-*-* } 0 }
+}
+
+// { dg-prune-output "use of deleted function" }
diff --git a/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc b/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc
index 5a8c3033575..cd850fdc21b 100644
--- a/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc
@@ -17,7 +17,6 @@
 
 // { dg-options "-fno-show-column" }
 // { dg-do compile { target c++14 } }
-// { dg-prune-output "tuple index is in range" }
 
 #include <tuple>
 
@@ -60,5 +59,7 @@ test03()
   std::get<6>(static_cast<test_type&&>(t));	// { dg-error "no match" }
 }
 
+// { dg-prune-output "tuple index must be in range" }
 // { dg-prune-output "no type named .type" }
 // { dg-prune-output "type/value mismatch" }
+// { dg-prune-output "use of deleted function" }

Re: [committed] libstdc++: Fix std::get<T> for std::tuple [PR101427]

[Jonathan Wakely]

On 15/07/21 16:26 +0100, Jonathan Wakely wrote:
>The std::get<T> functions relied on deduction failing if more than one
>base class existed for the type T.  However the implementation of Core
>DR 2303 (in r11-4693) made deduction succeed (and select the
>more-derived base class).
>
>This rewrites the implementation of std::get<T> to explicitly check for
>more than one occurrence of T in the tuple elements, making it
>ill-formed again. Additionally, the large wall of overload resolution
>errors described in PR c++/101460 is avoided by making std::get<T> use
>__get_helper<I> directly instead of calling std::get<I>, and by adding a
>deleted overload of __get_helper<N> for out-of-range N.
>
>Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>
>libstdc++-v3/ChangeLog:
>
>	PR libstdc++/101427
>	* include/std/tuple (tuple_element): Improve static_assert text.
>	(__get_helper): Add deleted overload.
>	(get<i>(tuple<T...>&&), get<i>(const tuple<T...>&&)): Use
>	__get_helper directly.
>	(__get_helper2): Remove.
>	(__find_uniq_type_in_pack): New constexpr helper function.
>	(get<T>): Use __find_uniq_type_in_pack and __get_helper instead
>	of __get_helper2.
>	* testsuite/20_util/tuple/element_access/get_neg.cc: Adjust
>	expected errors.
>	* testsuite/20_util/tuple/element_access/101427.cc: New test.
>
>Tested powerpc64le-linux. Committed to trunk.

This should be backported to gcc-11 in time for 11.2 as well. If you
see any problems with it please let me know ASAP.

[committed] libstdc++: Improve diagnostics for std::get with invalid tuple index

[Jonathan Wakely]

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

The recent fix for std::get<T> uses a deleted overload to give better
diagnostics for out-of-range indices. This does something similar for
std::get<I>.

Tested powerpc64le-linux. Committed to trunk.


This adds a deleted overload of std::get<I>(const tuple<Types...>&).
Invalid calls with an out of range index will match the deleted overload
and give a single, clear error about calling a deleted function, instead
of overload resolution errors for every std::get overload in the
library.

This changes the current output of 15+ errors (plus notes and associated
header context) into just two errors (plus context):

error: static assertion failed: tuple index must be in range
error: use of deleted function 'constexpr std::__enable_if_t<(__i >= sizeof... (_Types))> std::get(const std::tuple<_Types ...>&) [with long unsigned int __i = 1; _Elements = {int}; std::__enable_if_t<(__i >= sizeof... (_Types))> = void]'

This seems like a nice improvement, although PR c++/66968 means that
"_Types" is printed in the signature rather than "_Elements".

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

libstdc++-v3/ChangeLog:

         * include/std/tuple (get<I>): Add deleted overload for bad
         index.
         * testsuite/20_util/tuple/element_access/get_neg.cc: Adjust
         expected errors.





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

commit 3dbc7b809a62167b36f217ab5f43207be19e5908
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jul 16 20:59:43 2021

    libstdc++: Improve diagnostics for std::get with invalid tuple index
    
    This adds a deleted overload of std::get<I>(const tuple<Types...>&).
    Invalid calls with an out of range index will match the deleted overload
    and give a single, clear error about calling a deleted function, instead
    of overload resolution errors for every std::get overload in the
    library.
    
    This changes the current output of 15+ errors (plus notes and associated
    header context) into just two errors (plus context):
    
    error: static assertion failed: tuple index must be in range
    error: use of deleted function 'constexpr std::__enable_if_t<(__i >= sizeof... (_Types))> std::get(const std::tuple<_Types ...>&) [with long unsigned int __i = 1; _Elements = {int}; std::__enable_if_t<(__i >= sizeof... (_Types))> = void]'
    
    This seems like a nice improvement, although PR c++/66968 means that
    "_Types" is printed in the signature rather than "_Elements".
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * include/std/tuple (get<I>): Add deleted overload for bad
            index.
            * testsuite/20_util/tuple/element_access/get_neg.cc: Adjust
            expected errors.

diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 6953f8715d7..8ee0d2f1ef5 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -1406,6 +1406,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       return std::forward<const __element_type>(std::__get_helper<__i>(__t));
     }
 
+  /// @cond undocumented
+  // Deleted overload chosen for invalid indices.
+  template<size_t __i, typename... _Elements>
+    constexpr __enable_if_t<(__i >= sizeof...(_Elements))>
+    get(const tuple<_Elements...>&) = delete;
+  /// @endcond
+
 #if __cplusplus >= 201402L
 
 #define __cpp_lib_tuples_by_type 201304
diff --git a/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc b/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc
index cd850fdc21b..225bb6245a6 100644
--- a/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/tuple/element_access/get_neg.cc
@@ -25,12 +25,12 @@ test01()
 {
   using test_type = std::tuple<>;
   test_type t;
-  std::get<0>(t);				// { dg-error "no match" }
-  std::get<0>(const_cast<const test_type&>(t));	// { dg-error "no match" }
-  std::get<0>(static_cast<test_type&&>(t));	// { dg-error "no match" }
-  std::get<5>(t);				// { dg-error "no match" }
-  std::get<5>(const_cast<const test_type&>(t));	// { dg-error "no match" }
-  std::get<5>(static_cast<test_type&&>(t));	// { dg-error "no match" }
+  std::get<0>(t);				// { dg-error "deleted" }
+  std::get<0>(const_cast<const test_type&>(t));	// { dg-error "deleted" }
+  std::get<0>(static_cast<test_type&&>(t));	// { dg-error "deleted" }
+  std::get<5>(t);				// { dg-error "deleted" }
+  std::get<5>(const_cast<const test_type&>(t));	// { dg-error "deleted" }
+  std::get<5>(static_cast<test_type&&>(t));	// { dg-error "deleted" }
 }
 
 void
@@ -38,12 +38,12 @@ test02()
 {
   using test_type = std::tuple<int>;
   test_type t;
-  std::get<1>(t);				// { dg-error "no match" }
-  std::get<1>(const_cast<const test_type&>(t));	// { dg-error "no match" }
-  std::get<1>(static_cast<test_type&&>(t));	// { dg-error "no match" }
-  std::get<5>(t);				// { dg-error "no match" }
-  std::get<5>(const_cast<const test_type&>(t));	// { dg-error "no match" }
-  std::get<5>(static_cast<test_type&&>(t));	// { dg-error "no match" }
+  std::get<1>(t);				// { dg-error "deleted" }
+  std::get<1>(const_cast<const test_type&>(t));	// { dg-error "deleted" }
+  std::get<1>(static_cast<test_type&&>(t));	// { dg-error "deleted" }
+  std::get<5>(t);				// { dg-error "deleted" }
+  std::get<5>(const_cast<const test_type&>(t));	// { dg-error "deleted" }
+  std::get<5>(static_cast<test_type&&>(t));	// { dg-error "deleted" }
 }
 
 void
@@ -51,15 +51,12 @@ test03()
 {
   using test_type = std::tuple<int, int, int, int>;
   test_type t;
-  std::get<5>(t);				// { dg-error "no match" }
-  std::get<5>(const_cast<const test_type&>(t));	// { dg-error "no match" }
-  std::get<5>(static_cast<test_type&&>(t));	// { dg-error "no match" }
-  std::get<6>(t);				// { dg-error "no match" }
-  std::get<6>(const_cast<const test_type&>(t));	// { dg-error "no match" }
-  std::get<6>(static_cast<test_type&&>(t));	// { dg-error "no match" }
+  std::get<5>(t);				// { dg-error "deleted" }
+  std::get<5>(const_cast<const test_type&>(t));	// { dg-error "deleted" }
+  std::get<5>(static_cast<test_type&&>(t));	// { dg-error "deleted" }
+  std::get<6>(t);				// { dg-error "deleted" }
+  std::get<6>(const_cast<const test_type&>(t));	// { dg-error "deleted" }
+  std::get<6>(static_cast<test_type&&>(t));	// { dg-error "deleted" }
 }
 
-// { dg-prune-output "tuple index must be in range" }
-// { dg-prune-output "no type named .type" }
-// { dg-prune-output "type/value mismatch" }
-// { dg-prune-output "use of deleted function" }
+// { dg-error "tuple index must be in range" "" { target *-*-* } 0 }