This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] RFC: Refactor std::tuple constraints


This refactors some std:tuple constraints, and fixes one bug. This
patch was generated with -b to ignore whitespace changes, so the
indentation is a little off (but it makes the patch smaller and easier
to read).

I haven't touched the std::tuple<T1, T2> partial specialization yet,
this is just a demonstration of what I'm proposing. I'd like to either
remove the partial specialization completely (as proposed the other
day) or refactor its constraints the same way.

One of the main changes is to use __enable_if_t instead of having to
say typename enable_if<...>::type everywhere.

In a few places I replaced the _Dummy template parameter (which makes
the constructor dependent) with a bool _NotEmpty parameter which is
actually used in the constraints. There's no need for a separate
_Dummy that way. That simplifies the _TCC alias:

-      template<typename _Dummy> using _TCC =
-        _TC<is_same<_Dummy, void>::value,
-            _Elements...>;
+      template<bool _Cond>
+	using _TCC = _TC<_Cond, _Elements...>;

_TC::_NotSameTuple is replaced with a __valid_args function returning
a bool that is used as the argument for the _TMC alias template:

-      template<typename... _UElements> using _TMC =
-                  _TC<(sizeof...(_Elements) == sizeof...(_UElements))
-		      && (_TC<(sizeof...(_UElements)==1), _Elements...>::
-			  template _NotSameTuple<_UElements...>()),
-                      _Elements...>;
+      template<typename... _UElements>
+	using _TMC = _TC<__valid_args<_UElements...>(), _Elements...>;

This avoids instantiating one specialization of _TC just to call
_NotSameTuple in order to find out the bool argument for another
specialization of _TC.

The __valid_args function checks the pack sizes so several explicit
checks for sizeof...(_Elements) >= 1 can be removed.

The _TNTC alias can be removed completely, replaced by _TCC<_Size1>,
where _Size1 is a bool parameter that replaces a _Dummy parameter.

The "allocator-extended default constructor" was missing any
constraints on constructibility, so I added that (that's the bug fix).

I've reported an LWG issue that the "allocator-extended default
constructor" is not conditionally explicit. The patch doesn't add
that.

I'd also like to give _TC, _TMC, _TCC etc. more descriptive names,
make them all private, and rename _Elements to _Types and _UElements
to _UTypes (consistent with the standard) but that can be done in a
separate patch.

I think this is slightly simpler and easier to maintain, but I'd like
to hear other views, especially from Ville who added most of these
constraints and does most of the work to maintain std::tuple.



diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 56b97c25eed..05fce2e8cd0 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -469,13 +469,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                     __not_<is_constructible<_Elements..., _SrcTuple>>
              >::value;
    }
-
-    template<typename... _UElements>
-    static constexpr bool _NotSameTuple()
-    {
-      return  __not_<is_same<tuple<_Elements...>,
-			     __remove_cvref_t<_UElements>...>>::value;
-    }
  };

  template<typename... _Elements>
@@ -510,12 +503,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    {
      return true;
    }
-
-    template<typename... _UElements>
-    static constexpr bool _NotSameTuple()
-    {
-      return true;
-    }
  };

  /// Primary class template, tuple
@@ -533,6 +520,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        {
          return __and_<is_default_constructible<_Elements>...>::value;
        }
+
        static constexpr bool _ImplicitlyDefaultConstructibleTuple()
        {
          return __and_<__is_implicitly_default_constructible<_Elements>...>
@@ -553,87 +541,86 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
	    __and_<is_nothrow_assignable<_Elements&, _UElements>...>::value;
	}

+      // Constraint for tuple(_UTypes&&...) where sizeof...(_UTypes) == 1.
+      template<typename _Up>
+	static constexpr bool __valid_args()
+	{
+	  return sizeof...(_Elements) == 1
+	    && !is_same<tuple, __remove_cvref_t<_Up>>::value;
+	}
+
+      // Constraint for tuple(_UTypes&&...) where sizeof...(_UTypes) > 1.
+      template<typename, typename, typename... _Tail>
+	static constexpr bool __valid_args()
+	{ return (sizeof...(_Tail) + 2) == sizeof...(_Elements); }
+
    public:
      template<typename _Dummy = void,
-               typename enable_if<_TC2<_Dummy>::
+               __enable_if_t<_TC2<_Dummy>::
			       _ImplicitlyDefaultConstructibleTuple(),
-                                  bool>::type = true>
+			     bool> = true>
	constexpr tuple()
	: _Inherited() { }

      template<typename _Dummy = void,
-               typename enable_if<_TC2<_Dummy>::
+               __enable_if_t<_TC2<_Dummy>::
			       _DefaultConstructibleTuple()
-                                  &&
-                                  !_TC2<_Dummy>::
+			     && !_TC2<_Dummy>::
				_ImplicitlyDefaultConstructibleTuple(),
-                                  bool>::type = false>
+			     bool> = false>
	explicit constexpr tuple()
	: _Inherited() { }

      // Shortcut for the cases where constructors taking _Elements...
      // need to be constrained.
-      template<typename _Dummy> using _TCC =
-        _TC<is_same<_Dummy, void>::value,
-            _Elements...>;
+      template<bool _Cond>
+	using _TCC = _TC<_Cond, _Elements...>;

-      template<typename _Dummy = void,
-               typename enable_if<
-                 _TCC<_Dummy>::template
+      template<bool _NotEmpty = (sizeof...(_Elements) >= 1),
+               __enable_if_t<_TCC<_NotEmpty>::template
			       _ConstructibleTuple<_Elements...>()
-                 && _TCC<_Dummy>::template
-                   _ImplicitlyConvertibleTuple<_Elements...>()
-                 && (sizeof...(_Elements) >= 1),
-               bool>::type=true>
+			     && _TCC<_NotEmpty>::template
+			       _ImplicitlyConvertibleTuple<_Elements...>(),
+			     bool> = true>
	constexpr tuple(const _Elements&... __elements)
	: _Inherited(__elements...) { }

-      template<typename _Dummy = void,
-               typename enable_if<
-                 _TCC<_Dummy>::template
+      template<bool _NotEmpty = (sizeof...(_Elements) >= 1),
+               __enable_if_t<_TCC<_NotEmpty>::template
			       _ConstructibleTuple<_Elements...>()
-                 && !_TCC<_Dummy>::template
-                   _ImplicitlyConvertibleTuple<_Elements...>()
-                 && (sizeof...(_Elements) >= 1),
-               bool>::type=false>
+			     && !_TCC<_NotEmpty>::template
+			       _ImplicitlyConvertibleTuple<_Elements...>(),
+			     bool> = false>
	explicit constexpr tuple(const _Elements&... __elements)
	: _Inherited(__elements...) { }

      // Shortcut for the cases where constructors taking _UElements...
      // need to be constrained.
-      template<typename... _UElements> using _TMC =
-                  _TC<(sizeof...(_Elements) == sizeof...(_UElements))
-		      && (_TC<(sizeof...(_UElements)==1), _Elements...>::
-			  template _NotSameTuple<_UElements...>()),
-                      _Elements...>;
+      template<typename... _UElements>
+	using _TMC = _TC<__valid_args<_UElements...>(), _Elements...>;

      // Shortcut for the cases where constructors taking tuple<_UElements...>
      // need to be constrained.
-      template<typename... _UElements> using _TMCT =
-                  _TC<(sizeof...(_Elements) == sizeof...(_UElements))
-		      && !is_same<tuple<_Elements...>,
-				  tuple<_UElements...>>::value,
+      template<typename... _UElements>
+	using _TMCT = _TC<(sizeof...(_Elements) == sizeof...(_UElements))
+			   && !is_same<tuple, tuple<_UElements...>>::value,
			   _Elements...>;

-      template<typename... _UElements, typename
-	       enable_if<
-		  _TMC<_UElements...>::template
+      template<typename... _UElements,
+	       __enable_if_t<_TMC<_UElements...>::template
			       _MoveConstructibleTuple<_UElements...>()
			     && _TMC<_UElements...>::template
-                    _ImplicitlyMoveConvertibleTuple<_UElements...>()
-                  && (sizeof...(_Elements) >= 1),
-        bool>::type=true>
+			       _ImplicitlyMoveConvertibleTuple<_UElements...>(),
+			     bool> = true>
	constexpr tuple(_UElements&&... __elements)
	: _Inherited(std::forward<_UElements>(__elements)...) { }

-      template<typename... _UElements, typename
-        enable_if<
-		  _TMC<_UElements...>::template
+      template<typename... _UElements,
+	       __enable_if_t<_TMC<_UElements...>::template
			       _MoveConstructibleTuple<_UElements...>()
			     && !_TMC<_UElements...>::template
-                    _ImplicitlyMoveConvertibleTuple<_UElements...>()
-                  && (sizeof...(_Elements) >= 1),
-        bool>::type=false>
+			       _ImplicitlyMoveConvertibleTuple<_UElements...>(),
+			     bool> = false>
	explicit constexpr tuple(_UElements&&... __elements)
	: _Inherited(std::forward<_UElements>(__elements)...) {	}

@@ -641,103 +628,101 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION

      constexpr tuple(tuple&&) = default;

-      // Shortcut for the cases where constructors taking tuples
-      // must avoid creating temporaries.
-      template<typename _Dummy> using _TNTC =
-        _TC<is_same<_Dummy, void>::value && sizeof...(_Elements) == 1,
-            _Elements...>;
-
-      template<typename... _UElements, typename _Dummy = void, typename
-        enable_if<_TMCT<_UElements...>::template
+      template<typename... _UElements,
+	       bool _Size1 = (sizeof...(_Elements) == 1),
+	       __enable_if_t<_TMCT<_UElements...>::template
			       _ConstructibleTuple<_UElements...>()
			     && _TMCT<_UElements...>::template
			       _ImplicitlyConvertibleTuple<_UElements...>()
-                  && _TNTC<_Dummy>::template
+			     && _TCC<_Size1>::template
			       _NonNestedTuple<const tuple<_UElements...>&>(),
-        bool>::type=true>
+			     bool> = true>
	constexpr tuple(const tuple<_UElements...>& __in)
	: _Inherited(static_cast<const _Tuple_impl<0, _UElements...>&>(__in))
	{ }

-      template<typename... _UElements, typename _Dummy = void, typename
-        enable_if<_TMCT<_UElements...>::template
+      template<typename... _UElements,
+	       bool _Size1 = (sizeof...(_Elements) == 1),
+	       __enable_if_t<_TMCT<_UElements...>::template
			       _ConstructibleTuple<_UElements...>()
			     && !_TMCT<_UElements...>::template
			       _ImplicitlyConvertibleTuple<_UElements...>()
-                  && _TNTC<_Dummy>::template
+			     && _TCC<_Size1>::template
			       _NonNestedTuple<const tuple<_UElements...>&>(),
-        bool>::type=false>
+			     bool> = false>
	explicit constexpr tuple(const tuple<_UElements...>& __in)
	: _Inherited(static_cast<const _Tuple_impl<0, _UElements...>&>(__in))
	{ }

-      template<typename... _UElements, typename _Dummy = void, typename
-        enable_if<_TMCT<_UElements...>::template
+      template<typename... _UElements,
+	       bool _Size1 = (sizeof...(_Elements) == 1),
+	       __enable_if_t<_TMCT<_UElements...>::template
			       _MoveConstructibleTuple<_UElements...>()
			     && _TMCT<_UElements...>::template
			       _ImplicitlyMoveConvertibleTuple<_UElements...>()
-                  && _TNTC<_Dummy>::template
+			     && _TCC<_Size1>::template
			       _NonNestedTuple<tuple<_UElements...>&&>(),
-        bool>::type=true>
+			     bool> = true>
	constexpr tuple(tuple<_UElements...>&& __in)
	: _Inherited(static_cast<_Tuple_impl<0, _UElements...>&&>(__in)) { }

-      template<typename... _UElements, typename _Dummy = void, typename
-        enable_if<_TMCT<_UElements...>::template
+      template<typename... _UElements,
+	       bool _Size1 = (sizeof...(_Elements) == 1),
+	       __enable_if_t<_TMCT<_UElements...>::template
			       _MoveConstructibleTuple<_UElements...>()
			     && !_TMCT<_UElements...>::template
			       _ImplicitlyMoveConvertibleTuple<_UElements...>()
-                  && _TNTC<_Dummy>::template
+			     && _TCC<_Size1>::template
			       _NonNestedTuple<tuple<_UElements...>&&>(),
-        bool>::type=false>
+			     bool> = false>
	explicit constexpr tuple(tuple<_UElements...>&& __in)
	: _Inherited(static_cast<_Tuple_impl<0, _UElements...>&&>(__in)) { }

      // Allocator-extended constructors.

-      template<typename _Alloc>
+      template<typename _Alloc,
+	       __enable_if_t<_TC2<_Alloc>::_DefaultConstructibleTuple(),
+			     bool> = true>
	tuple(allocator_arg_t __tag, const _Alloc& __a)
	: _Inherited(__tag, __a) { }

-      template<typename _Alloc, typename _Dummy = void,
-               typename enable_if<
-                 _TCC<_Dummy>::template
+      template<typename _Alloc, bool _NotEmpty = (sizeof...(_Elements) >= 1),
+	       __enable_if_t<_TCC<_NotEmpty>::template
			       _ConstructibleTuple<_Elements...>()
-                 && _TCC<_Dummy>::template
+			     && _TCC<_NotEmpty>::template
			       _ImplicitlyConvertibleTuple<_Elements...>(),
-               bool>::type=true>
+			     bool> = true>
	tuple(allocator_arg_t __tag, const _Alloc& __a,
	      const _Elements&... __elements)
	: _Inherited(__tag, __a, __elements...) { }

-      template<typename _Alloc, typename _Dummy = void,
-               typename enable_if<
-                 _TCC<_Dummy>::template
+      template<typename _Alloc, bool _NotEmpty = (sizeof...(_Elements) >= 1),
+               __enable_if_t<_TCC<_NotEmpty>::template
			       _ConstructibleTuple<_Elements...>()
-                 && !_TCC<_Dummy>::template
+			     && !_TCC<_NotEmpty>::template
			       _ImplicitlyConvertibleTuple<_Elements...>(),
-               bool>::type=false>
+			     bool> = false>
	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
	      const _Elements&... __elements)
	: _Inherited(__tag, __a, __elements...) { }

-      template<typename _Alloc, typename... _UElements, typename
-        enable_if<_TMC<_UElements...>::template
+      template<typename _Alloc, typename... _UElements,
+	       __enable_if_t<_TMC<_UElements...>::template
			       _MoveConstructibleTuple<_UElements...>()
			     && _TMC<_UElements...>::template
			       _ImplicitlyMoveConvertibleTuple<_UElements...>(),
-        bool>::type=true>
+			     bool> = true>
	tuple(allocator_arg_t __tag, const _Alloc& __a,
	      _UElements&&... __elements)
	: _Inherited(__tag, __a, std::forward<_UElements>(__elements)...)
       	{ }

-      template<typename _Alloc, typename... _UElements, typename
-        enable_if<_TMC<_UElements...>::template
+      template<typename _Alloc, typename... _UElements,
+	       __enable_if_t<_TMC<_UElements...>::template
			       _MoveConstructibleTuple<_UElements...>()
			     && !_TMC<_UElements...>::template
			       _ImplicitlyMoveConvertibleTuple<_UElements...>(),
-        bool>::type=false>
+			     bool> = false>
	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
	      _UElements&&... __elements)
	: _Inherited(__tag, __a, std::forward<_UElements>(__elements)...)
@@ -751,60 +736,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
	tuple(allocator_arg_t __tag, const _Alloc& __a, tuple&& __in)
	: _Inherited(__tag, __a, static_cast<_Inherited&&>(__in)) { }

-      template<typename _Alloc, typename _Dummy = void,
-	       typename... _UElements, typename
-        enable_if<_TMCT<_UElements...>::template
+      template<typename _Alloc, typename... _UElements,
+	       bool _Size1 = (sizeof...(_Elements) == 1),
+	       __enable_if_t<_TMCT<_UElements...>::template
			       _ConstructibleTuple<_UElements...>()
			     && _TMCT<_UElements...>::template
			       _ImplicitlyConvertibleTuple<_UElements...>()
-                  && _TNTC<_Dummy>::template
+			     && _TCC<_Size1>::template
			       _NonNestedTuple<tuple<_UElements...>&&>(),
-        bool>::type=true>
+			     bool> = true>
	tuple(allocator_arg_t __tag, const _Alloc& __a,
	      const tuple<_UElements...>& __in)
	: _Inherited(__tag, __a,
	             static_cast<const _Tuple_impl<0, _UElements...>&>(__in))
	{ }

-      template<typename _Alloc, typename _Dummy = void,
-	       typename... _UElements, typename
-        enable_if<_TMCT<_UElements...>::template
+      template<typename _Alloc, typename... _UElements,
+	       bool _Size1 = (sizeof...(_Elements) == 1),
+	       __enable_if_t<_TMCT<_UElements...>::template
			       _ConstructibleTuple<_UElements...>()
			     && !_TMCT<_UElements...>::template
			       _ImplicitlyConvertibleTuple<_UElements...>()
-                  && _TNTC<_Dummy>::template
+			     && _TCC<_Size1>::template
			       _NonNestedTuple<tuple<_UElements...>&&>(),
-        bool>::type=false>
+			     bool> = false>
	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
	      const tuple<_UElements...>& __in)
	: _Inherited(__tag, __a,
	             static_cast<const _Tuple_impl<0, _UElements...>&>(__in))
	{ }

-      template<typename _Alloc, typename _Dummy = void,
-	       typename... _UElements, typename
-        enable_if<_TMCT<_UElements...>::template
+      template<typename _Alloc, typename... _UElements,
+	       bool _Size1 = (sizeof...(_Elements) == 1),
+	       __enable_if_t<_TMCT<_UElements...>::template
			       _MoveConstructibleTuple<_UElements...>()
			     && _TMCT<_UElements...>::template
			       _ImplicitlyMoveConvertibleTuple<_UElements...>()
-                  && _TNTC<_Dummy>::template
+			     && _TCC<_Size1>::template
			       _NonNestedTuple<tuple<_UElements...>&&>(),
-        bool>::type=true>
+			     bool> = true>
	tuple(allocator_arg_t __tag, const _Alloc& __a,
	      tuple<_UElements...>&& __in)
	: _Inherited(__tag, __a,
	             static_cast<_Tuple_impl<0, _UElements...>&&>(__in))
	{ }

-      template<typename _Alloc, typename _Dummy = void,
-	       typename... _UElements, typename
-        enable_if<_TMCT<_UElements...>::template
+      template<typename _Alloc, typename... _UElements,
+	       bool _Size1 = (sizeof...(_Elements) == 1),
+	       __enable_if_t<_TMCT<_UElements...>::template
			       _MoveConstructibleTuple<_UElements...>()
			     && !_TMCT<_UElements...>::template
			       _ImplicitlyMoveConvertibleTuple<_UElements...>()
-                  && _TNTC<_Dummy>::template
+			     && _TCC<_Size1>::template
			       _NonNestedTuple<tuple<_UElements...>&&>(),
-        bool>::type=false>
+			     bool> = false>
	explicit tuple(allocator_arg_t __tag, const _Alloc& __a,
	      tuple<_UElements...>&& __in)
	: _Inherited(__tag, __a,


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]