diff mbox

Simplify std::tuple helpers and fix C++14 bug.

Message ID 20140514222236.GG10556@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely May 14, 2014, 10:22 p.m. UTC
I found a bug in the new std::get<Tp>(tuple<Types...>&&) function
where it didn't compile when trying to access reference member.

The rest of this patch simplifies the code in <tuple> by:

 - removing the redundant __add_ref etc. helpers.
 - defining a __tuple_element_t alias template.
 - removing duplication in the tuple_size<cv T> traits.
 - using static_assert in the relational operators.

Tested x86_64-linux, committed to trunk.

Comments

Daniel Krügler May 15, 2014, 5:36 a.m. UTC | #1
2014-05-15 0:22 GMT+02:00 Jonathan Wakely <jwakely@redhat.com>:
> I found a bug in the new std::get<Tp>(tuple<Types...>&&) function
> where it didn't compile when trying to access reference member.
>
> The rest of this patch simplifies the code in <tuple> by:
>
> - removing the redundant __add_ref etc. helpers.
> - defining a __tuple_element_t alias template.
> - removing duplication in the tuple_size<cv T> traits.
> - using static_assert in the relational operators.

Looking at the definition of the new alias

__cv_tuple_size

you might want to apply LWG 2313

http://cplusplus.github.io/LWG/lwg-defects.html#2313

and simplify its definition even further.

- Daniel
diff mbox

Patch

commit 61f1eec0e42041d550f3eee1626950f7861255e6
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 13 13:53:23 2014 +0100

    	* include/std/tuple (__add_c_ref, __add_ref, __add_r_ref): Remove.
    	(__tuple_element_t): Define.
    	(tuple_element): Use __tuple_element_t.
    	(__cv_tuple_size): Define.
    	(tuple_size<cv _Tp>): Use __cv_tuple_size.
    	(get, __get_helper, __get_helper2): Remove uses of __add_ref etc.
    	(get<_Tp>(tuple<_Types...>&&)): Use forward instead of move.
    	(__tuple_compare): Remove size check, re-order parameters.
    	(operator==, operator<): Use static_assert to check requirements.
    	* include/std/functional (__volget): use __tuple_element_t.
    	* testsuite/20_util/tuple/element_access/get_by_type.cc: Test rvalues.
    	* testsuite/20_util/uses_allocator/cons_neg.cc: Adjust dg-error.

diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
index 2bc3d8d..e677c24 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -1230,14 +1230,14 @@  _GLIBCXX_HAS_NESTED_TYPE(result_type)
   template<std::size_t _Ind, typename... _Tp>
     inline auto
     __volget(volatile tuple<_Tp...>& __tuple)
-    -> typename tuple_element<_Ind, tuple<_Tp...>>::type volatile&
+    -> __tuple_element_t<_Ind, tuple<_Tp...>> volatile&
     { return std::get<_Ind>(const_cast<tuple<_Tp...>&>(__tuple)); }
 
   // std::get<I> for const-volatile-qualified tuples
   template<std::size_t _Ind, typename... _Tp>
     inline auto
     __volget(const volatile tuple<_Tp...>& __tuple)
-    -> typename tuple_element<_Ind, tuple<_Tp...>>::type const volatile&
+    -> __tuple_element_t<_Ind, tuple<_Tp...>> const volatile&
     { return std::get<_Ind>(const_cast<const tuple<_Tp...>&>(__tuple)); }
 
   /// Type of the function object returned from bind().
diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 5e8766c..95c197d 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -48,33 +48,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
    *  @{
    */
 
-  // Adds a const reference to a non-reference type.
-  template<typename _Tp>
-    struct __add_c_ref
-    { typedef const _Tp& type; };
-
-  template<typename _Tp>
-    struct __add_c_ref<_Tp&>
-    { typedef _Tp& type; };
-
-  // Adds a reference to a non-reference type.
-  template<typename _Tp>
-    struct __add_ref
-    { typedef _Tp& type; };
-
-  template<typename _Tp>
-    struct __add_ref<_Tp&>
-    { typedef _Tp& type; };
-
-  // Adds an rvalue reference to a non-reference type.
-  template<typename _Tp>
-    struct __add_r_ref
-    { typedef _Tp&& type; };
-
-  template<typename _Tp>
-    struct __add_r_ref<_Tp&>
-    { typedef _Tp& type; };
-
   template<std::size_t _Idx, typename _Head, bool _IsEmptyNotFinal>
     struct _Head_base;
 
@@ -689,25 +662,26 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       typedef _Head type;
     };
 
+  // Duplicate of C++14's tuple_element_t for internal use in C++11 mode
+  template<std::size_t __i, typename _Tp>
+    using __tuple_element_t = typename tuple_element<__i, _Tp>::type;
+
   template<std::size_t __i, typename _Tp>
     struct tuple_element<__i, const _Tp>
     {
-      typedef typename
-      add_const<typename tuple_element<__i, _Tp>::type>::type type;
+      typedef typename add_const<__tuple_element_t<__i, _Tp>>::type type;
     };
 
   template<std::size_t __i, typename _Tp>
     struct tuple_element<__i, volatile _Tp>
     {
-      typedef typename
-      add_volatile<typename tuple_element<__i, _Tp>::type>::type type;
+      typedef typename add_volatile<__tuple_element_t<__i, _Tp>>::type type;
     };
 
   template<std::size_t __i, typename _Tp>
     struct tuple_element<__i, const volatile _Tp>
     {
-      typedef typename
-      add_cv<typename tuple_element<__i, _Tp>::type>::type type;
+      typedef typename add_cv<__tuple_element_t<__i, _Tp>>::type type;
     };
 
 #if __cplusplus > 201103L
@@ -719,23 +693,18 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Tp>
     struct tuple_size;
 
+  template<typename _Tp, typename _Ts = tuple_size<_Tp>>
+    using __cv_tuple_size = integral_constant<
+             typename remove_cv<decltype(_Ts::value)>::type, _Ts::value>;
+
   template<typename _Tp>
-    struct tuple_size<const _Tp>
-    : public integral_constant<
-             typename remove_cv<decltype(tuple_size<_Tp>::value)>::type,
-             tuple_size<_Tp>::value> { };
+    struct tuple_size<const _Tp> : __cv_tuple_size<_Tp> { };
 
   template<typename _Tp>
-    struct tuple_size<volatile _Tp>
-    : public integral_constant<
-             typename remove_cv<decltype(tuple_size<_Tp>::value)>::type,
-             tuple_size<_Tp>::value> { };
+    struct tuple_size<volatile _Tp> : __cv_tuple_size<_Tp> { };
 
   template<typename _Tp>
-    struct tuple_size<const volatile _Tp>
-    : public integral_constant<
-             typename remove_cv<decltype(tuple_size<_Tp>::value)>::type,
-             tuple_size<_Tp>::value> { };
+    struct tuple_size<const volatile _Tp> : __cv_tuple_size<_Tp> { };
 
   /// class tuple_size
   template<typename... _Elements>
@@ -743,98 +712,93 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     : public integral_constant<std::size_t, sizeof...(_Elements)> { };
 
   template<std::size_t __i, typename _Head, typename... _Tail>
-    constexpr typename __add_ref<_Head>::type
+    constexpr _Head&
     __get_helper(_Tuple_impl<__i, _Head, _Tail...>& __t) noexcept
     { return _Tuple_impl<__i, _Head, _Tail...>::_M_head(__t); }
 
   template<std::size_t __i, typename _Head, typename... _Tail>
-    constexpr typename __add_c_ref<_Head>::type
+    constexpr const _Head&
     __get_helper(const _Tuple_impl<__i, _Head, _Tail...>& __t) noexcept
     { return _Tuple_impl<__i, _Head, _Tail...>::_M_head(__t); }
 
-  // Return a reference (const reference, rvalue reference) to the ith element
-  // of a tuple.  Any const or non-const ref elements are returned with their
-  // original type.
+  /// Return a reference to the ith element of a tuple.
   template<std::size_t __i, typename... _Elements>
-    constexpr typename __add_ref<
-                      typename tuple_element<__i, tuple<_Elements...>>::type
-                    >::type
+    constexpr __tuple_element_t<__i, tuple<_Elements...>>&
     get(tuple<_Elements...>& __t) noexcept
     { return std::__get_helper<__i>(__t); }
 
+  /// Return a const reference to the ith element of a const tuple.
   template<std::size_t __i, typename... _Elements>
-    constexpr typename __add_c_ref<
-                      typename tuple_element<__i, tuple<_Elements...>>::type
-                    >::type
+    constexpr const __tuple_element_t<__i, tuple<_Elements...>>&
     get(const tuple<_Elements...>& __t) noexcept
     { return std::__get_helper<__i>(__t); }
 
+  /// Return an rvalue reference to the ith element of a tuple rvalue.
   template<std::size_t __i, typename... _Elements>
-    constexpr typename __add_r_ref<
-                      typename tuple_element<__i, tuple<_Elements...>>::type
-                    >::type
+    constexpr __tuple_element_t<__i, tuple<_Elements...>>&&
     get(tuple<_Elements...>&& __t) noexcept
-    { return std::forward<typename tuple_element<__i,
-	tuple<_Elements...>>::type&&>(std::get<__i>(__t)); }
+    {
+      typedef __tuple_element_t<__i, tuple<_Elements...>> __element_type;
+      return std::forward<__element_type&&>(std::get<__i>(__t));
+    }
 
 #if __cplusplus > 201103L
   template<typename _Head, size_t __i, typename... _Tail>
-    constexpr typename __add_ref<_Head>::type
+    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 typename __add_c_ref<_Head>::type
+    constexpr const _Head&
     __get_helper2(const _Tuple_impl<__i, _Head, _Tail...>& __t) noexcept
     { return _Tuple_impl<__i, _Head, _Tail...>::_M_head(__t); }
 
+  /// 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); }
 
+  /// 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::move(std::__get_helper2<_Tp>(__t)); }
+    { return std::forward<_Tp&&>(std::__get_helper2<_Tp>(__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); }
 #endif
 
-  // This class helps construct the various comparison operations on tuples
-  template<std::size_t __check_equal_size, std::size_t __i, std::size_t __j,
-	   typename _Tp, typename _Up>
-    struct __tuple_compare;
-
-  template<std::size_t __i, std::size_t __j, typename _Tp, typename _Up>
-    struct __tuple_compare<0, __i, __j, _Tp, _Up>
+  // This class performs the comparison operations on tuples
+  template<typename _Tp, typename _Up, size_t __i, size_t __size>
+    struct __tuple_compare
     {
-      static constexpr bool 
+      static constexpr bool
       __eq(const _Tp& __t, const _Up& __u)
       {
 	return bool(std::get<__i>(__t) == std::get<__i>(__u))
-	  && __tuple_compare<0, __i + 1, __j, _Tp, _Up>::__eq(__t, __u);
+	  && __tuple_compare<_Tp, _Up, __i + 1, __size>::__eq(__t, __u);
       }
-     
-      static constexpr bool 
+   
+      static constexpr bool
       __less(const _Tp& __t, const _Up& __u)
       {
 	return bool(std::get<__i>(__t) < std::get<__i>(__u))
-	  || !bool(std::get<__i>(__u) < std::get<__i>(__t))
-	  && __tuple_compare<0, __i + 1, __j, _Tp, _Up>::__less(__t, __u);
+	  || (!bool(std::get<__i>(__u) < std::get<__i>(__t))
+	      && __tuple_compare<_Tp, _Up, __i + 1, __size>::__less(__t, __u));
       }
     };
 
-  template<std::size_t __i, typename _Tp, typename _Up>
-    struct __tuple_compare<0, __i, __i, _Tp, _Up>
+  template<typename _Tp, typename _Up, size_t __size>
+    struct __tuple_compare<_Tp, _Up, __size, __size>
     {
-      static constexpr bool 
+      static constexpr bool
       __eq(const _Tp&, const _Up&) { return true; }
-     
-      static constexpr bool 
+   
+      static constexpr bool
       __less(const _Tp&, const _Up&) { return false; }
     };
 
@@ -843,10 +807,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator==(const tuple<_TElements...>& __t,
 	       const tuple<_UElements...>& __u)
     {
-      typedef tuple<_TElements...> _Tp;
-      typedef tuple<_UElements...> _Up;
-      return bool(__tuple_compare<tuple_size<_Tp>::value - tuple_size<_Up>::value,
-	      0, tuple_size<_Tp>::value, _Tp, _Up>::__eq(__t, __u));
+      static_assert(sizeof...(_TElements) == sizeof...(_UElements),
+	  "tuple objects can only be compared if they have equal sizes.");
+      using __compare = __tuple_compare<tuple<_TElements...>,
+					tuple<_UElements...>,
+					0, sizeof...(_TElements)>;
+      return __compare::__eq(__t, __u);
     }
 
   template<typename... _TElements, typename... _UElements>
@@ -854,10 +820,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator<(const tuple<_TElements...>& __t,
 	      const tuple<_UElements...>& __u)
     {
-      typedef tuple<_TElements...> _Tp;
-      typedef tuple<_UElements...> _Up;
-      return bool(__tuple_compare<tuple_size<_Tp>::value - tuple_size<_Up>::value,
-	      0, tuple_size<_Tp>::value, _Tp, _Up>::__less(__t, __u));
+      static_assert(sizeof...(_TElements) == sizeof...(_UElements),
+	  "tuple objects can only be compared if they have equal sizes.");
+      using __compare = __tuple_compare<tuple<_TElements...>,
+					tuple<_UElements...>,
+					0, sizeof...(_TElements)>;
+      return __compare::__less(__t, __u);
     }
 
   template<typename... _TElements, typename... _UElements>
@@ -922,17 +890,15 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
             <typename std::remove_reference<_Tp>::type>::type>::type
     { };
 
-  template<std::size_t, typename, typename, std::size_t>
+  template<size_t, typename, typename, size_t>
     struct __make_tuple_impl;
 
-  template<std::size_t _Idx, typename _Tuple, typename... _Tp,
-           std::size_t _Nm>
+  template<size_t _Idx, typename _Tuple, typename... _Tp, size_t _Nm>
     struct __make_tuple_impl<_Idx, tuple<_Tp...>, _Tuple, _Nm>
-    {
-      typedef typename __make_tuple_impl<_Idx + 1, tuple<_Tp...,
-	typename std::tuple_element<_Idx, _Tuple>::type>, _Tuple, _Nm>::__type
-      __type;
-    };
+    : __make_tuple_impl<_Idx + 1,
+			tuple<_Tp..., __tuple_element_t<_Idx, _Tuple>>,
+			_Tuple, _Nm>
+    { };
 
   template<std::size_t _Nm, typename _Tuple, typename... _Tp>
     struct __make_tuple_impl<_Nm, tuple<_Tp...>, _Tuple, _Nm>
@@ -942,8 +908,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   template<typename _Tuple>
     struct __do_make_tuple
-    : public __make_tuple_impl<0, tuple<>, _Tuple,
-                               std::tuple_size<_Tuple>::value>
+    : __make_tuple_impl<0, tuple<>, _Tuple, std::tuple_size<_Tuple>::value>
     { };
 
   // Returns the std::tuple equivalent of a tuple-like type.
diff --git a/libstdc++-v3/testsuite/20_util/tuple/element_access/get_by_type.cc b/libstdc++-v3/testsuite/20_util/tuple/element_access/get_by_type.cc
index 226e9e4..042f214 100644
--- a/libstdc++-v3/testsuite/20_util/tuple/element_access/get_by_type.cc
+++ b/libstdc++-v3/testsuite/20_util/tuple/element_access/get_by_type.cc
@@ -41,4 +41,8 @@  main()
   get<1>(b)=5;
   VERIFY(get<int>(b)==1 && get<int&>(b)==5 && get<const int&>(b)==2);
   VERIFY(j==5);
+  // test rvalue overload:
+  VERIFY(get<int>(std::move(b))==1);
+  VERIFY(get<int&>(std::move(b))==5);
+  VERIFY(get<const int&>(std::move(b))==2);
 }
diff --git a/libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc b/libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc
index 5ce344c..cf5d6a57 100644
--- a/libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc
@@ -44,4 +44,4 @@  void test01()
 
   tuple<Type> t(allocator_arg, a, 1);
 }
-// { dg-error "no matching function" "" { target *-*-* } 118 }
+// { dg-error "no matching function" "" { target *-*-* } 91 }