[RFC] Deprecate non-standard constructors in std::pair

Jonathan Wakely jwakely@redhat.com
Wed Apr 28 16:57:57 GMT 2021


On 07/04/21 18:18 +0100, Jonathan Wakely wrote:
>On 07/04/21 17:59 +0100, Jonathan Wakely wrote:
>>On 07/04/21 13:46 +0100, Jonathan Wakely wrote:
>>>On 07/04/21 15:41 +0300, Ville Voutilainen via Libstdc++ wrote:
>>>>On Wed, 7 Apr 2021 at 15:31, Jonathan Wakely via Libstdc++
>>>><libstdc++@gcc.gnu.org> wrote:
>>>>>I propose that we deprecate the constructors for C++11/14/17/20 in
>>>>>stage 1, and do not support them at all in C++23 mode once P1951 is
>>>>>supported. I have a patch which I'll send in stage 1 (it also uses
>>>>>C++20 concepts to simplify std::pair and fix PR 97930).
>>>>>
>>>>>After a period of deprecation we could remove them, and support P1951
>>>>>for -std=gnu++11/14/17/20 too so that {} continues to work.
>>>>
>>>>The proposal sounds good to me.
>>>
>>>Thanks. I've created https://gcc.gnu.org/PR99957 so I don't forget.
>>
>>Here's a patch to implement it, for stage 1.
>
>And here's a patch to simplify the std::pair constraints using
>concepts, also for consideration in stage 1.

I've pushed this to trunk too, after testing on powerpc64le-linux.

>commit 9e184c70f6a8e8ed8e34d8ffcb9c98e079dd3c31
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date:   Wed Apr 7 18:12:08 2021
>
>    libstdc++: Simplify std::pair constraints using concepts
>    
>    This re-implements the constraints on the std::pair constructors and
>    assignment operators in C++20 mode, to use concepts.
>    
>    The non-standard constructors deprecated for PR 99957 are no longer
>    supported in C++20 mode, which requires some minor testsuite changes.
>    Otherwise all tests pass in C++20 mode.
>    
>    libstdc++-v3/ChangeLog:
>    
>            * include/bits/stl_pair.h (pair):
>            * testsuite/20_util/pair/cons/99957.cc: Disable for C++20 and
>            later.
>            * testsuite/20_util/pair/cons/explicit_construct.cc: Adjust
>            expected error messages to also match C++20 errors.
>
>diff --git a/libstdc++-v3/include/bits/stl_pair.h b/libstdc++-v3/include/bits/stl_pair.h
>index 883d7441b3d..bd3f911abb9 100644
>--- a/libstdc++-v3/include/bits/stl_pair.h
>+++ b/libstdc++-v3/include/bits/stl_pair.h
>@@ -92,6 +92,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   template<size_t...>
>     struct _Index_tuple;
> 
>+#if ! __cpp_lib_concepts
>   // Concept utility functions, reused in conditionally-explicit
>   // constructors.
>   // See PR 70437, don't look at is_constructible or
>@@ -171,11 +172,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	return false;
>       }
>     };
>+#endif // lib concepts
> #endif // C++11
> 
>   template<typename _U1, typename _U2> class __pair_base
>   {
>-#if __cplusplus >= 201103L
>+#if __cplusplus >= 201103L && ! __cpp_lib_concepts
>     template<typename _T1, typename _T2> friend struct pair;
>     __pair_base() = default;
>     ~__pair_base() = default;
>@@ -196,7 +198,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    */
>   template<typename _T1, typename _T2>
>     struct pair
>-    : private __pair_base<_T1, _T2>
>+    : public __pair_base<_T1, _T2>
>     {
>       typedef _T1 first_type;    ///< The type of the `first` member
>       typedef _T2 second_type;   ///< The type of the `second` member
>@@ -205,7 +207,186 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       _T2 second;                ///< The second member
> 
> #if __cplusplus >= 201103L
>-      // C++11 (and later) implementation.
>+      constexpr pair(const pair&) = default;	///< Copy constructor
>+      constexpr pair(pair&&) = default;		///< Move constructor
>+
>+      template<typename... _Args1, typename... _Args2>
>+	_GLIBCXX20_CONSTEXPR
>+	pair(piecewise_construct_t, tuple<_Args1...>, tuple<_Args2...>);
>+
>+      /// Swap the first members and then the second members.
>+      _GLIBCXX20_CONSTEXPR void
>+      swap(pair& __p)
>+      noexcept(__and_<__is_nothrow_swappable<_T1>,
>+		      __is_nothrow_swappable<_T2>>::value)
>+      {
>+	using std::swap;
>+	swap(first, __p.first);
>+	swap(second, __p.second);
>+      }
>+
>+    private:
>+      template<typename... _Args1, size_t... _Indexes1,
>+	       typename... _Args2, size_t... _Indexes2>
>+	_GLIBCXX20_CONSTEXPR
>+	pair(tuple<_Args1...>&, tuple<_Args2...>&,
>+	     _Index_tuple<_Indexes1...>, _Index_tuple<_Indexes2...>);
>+    public:
>+
>+#if __cpp_lib_concepts
>+      // C++20 implementation using concepts, explicit(bool), fully constexpr.
>+
>+      /// Default constructor
>+      constexpr
>+      explicit(__not_<__and_<__is_implicitly_default_constructible<_T1>,
>+			     __is_implicitly_default_constructible<_T2>>>())
>+      pair()
>+      requires is_default_constructible_v<_T1>
>+	       && is_default_constructible_v<_T2>
>+      : first(), second()
>+      { }
>+
>+    private:
>+
>+      /// @cond undocumented
>+      template<typename _U1, typename _U2>
>+	static constexpr bool
>+	_S_constructible()
>+	{
>+	  if constexpr (is_constructible_v<_T1, _U1>)
>+	    return is_constructible_v<_T2, _U2>;
>+	  return false;
>+	}
>+
>+      template<typename _U1, typename _U2>
>+	static constexpr bool
>+	_S_nothrow_constructible()
>+	{
>+	  if constexpr (is_nothrow_constructible_v<_T1, _U1>)
>+	    return is_nothrow_constructible_v<_T2, _U2>;
>+	  return false;
>+	}
>+
>+      template<typename _U1, typename _U2>
>+	static constexpr bool
>+	_S_convertible()
>+	{
>+	  if constexpr (is_convertible_v<_U1, _T1>)
>+	    return is_convertible_v<_U2, _T2>;
>+	  return false;
>+	}
>+      /// @endcond
>+
>+    public:
>+
>+      /// Constructor accepting lvalues of `first_type` and `second_type`
>+      constexpr explicit(!_S_convertible<const _T1&, const _T2&>())
>+      pair(const _T1& __x, const _T2& __y)
>+      noexcept(_S_nothrow_constructible<const _T1&, const _T2&>())
>+      requires (_S_constructible<const _T1&, const _T2&>())
>+      : first(__x), second(__y)
>+      { }
>+
>+      /// Constructor accepting two values of arbitrary types
>+      template<typename _U1, typename _U2>
>+	requires (_S_constructible<_U1, _U2>())
>+	constexpr explicit(!_S_convertible<_U1, _U2>())
>+	pair(_U1&& __x, _U2&& __y)
>+	noexcept(_S_nothrow_constructible<_U1, _U2>())
>+	: first(std::forward<_U1>(__x)), second(std::forward<_U2>(__y))
>+	{ }
>+
>+      /// Converting constructor from a `pair<U1, U2>` lvalue
>+      template<typename _U1, typename _U2>
>+	requires (_S_constructible<const _U1&, const _U2&>())
>+	constexpr explicit(!_S_convertible<const _U1&, const _U2&>())
>+	pair(const pair<_U1, _U2>& __p)
>+	noexcept(_S_nothrow_constructible<const _U1&, const _U2&>())
>+	: first(__p.first), second(__p.second)
>+	{ }
>+
>+      /// Converting constructor from a `pair<U1, U2>` rvalue
>+      template<typename _U1, typename _U2>
>+	requires (_S_constructible<_U1, _U2>())
>+	constexpr explicit(!_S_convertible<_U1, _U2>())
>+	pair(pair<_U1, _U2>&& __p)
>+	noexcept(_S_nothrow_constructible<_U1, _U2>())
>+	: first(std::forward<_U1>(__p.first)),
>+	  second(std::forward<_U2>(__p.second))
>+	{ }
>+
>+  private:
>+      /// @cond undocumented
>+      template<typename _U1, typename _U2>
>+	static constexpr bool
>+	_S_assignable()
>+	{
>+	  if constexpr (is_assignable_v<_T1&, _U1>)
>+	    return is_assignable_v<_T2&, _U2>;
>+	  return false;
>+	}
>+
>+      template<typename _U1, typename _U2>
>+	static constexpr bool
>+	_S_nothrow_assignable()
>+	{
>+	  if constexpr (is_nothrow_assignable_v<_T1&, _U1>)
>+	    return is_nothrow_assignable_v<_T2&, _U2>;
>+	  return false;
>+	}
>+      /// @endcond
>+
>+  public:
>+
>+      pair& operator=(const pair&) = delete;
>+
>+      /// Copy assignment operator
>+      constexpr pair&
>+      operator=(const pair& __p)
>+      noexcept(_S_nothrow_assignable<const _T1&, const _T2&>())
>+      requires (_S_assignable<const _T1&, const _T2&>())
>+      {
>+	first = __p.first;
>+	second = __p.second;
>+	return *this;
>+      }
>+
>+      /// Move assignment operator
>+      constexpr pair&
>+      operator=(pair&& __p)
>+      noexcept(_S_nothrow_assignable<_T1, _T2>())
>+      requires (_S_assignable<_T1, _T2>())
>+      {
>+	first = std::forward<first_type>(__p.first);
>+	second = std::forward<second_type>(__p.second);
>+	return *this;
>+      }
>+
>+      /// Converting assignment from a `pair<U1, U2>` lvalue
>+      template<typename _U1, typename _U2>
>+	constexpr pair&
>+	operator=(const pair<_U1, _U2>& __p)
>+	noexcept(_S_nothrow_assignable<const _U1&, const _U2&>())
>+	requires (_S_assignable<const _U1&, const _U2&>())
>+	{
>+	  first = __p.first;
>+	  second = __p.second;
>+	  return *this;
>+	}
>+
>+      /// Converting assignment from a `pair<U1, U2>` rvalue
>+      template<typename _U1, typename _U2>
>+	constexpr pair&
>+	operator=(pair<_U1, _U2>&& __p)
>+	noexcept(_S_nothrow_assignable<_U1, _U2>())
>+	requires (_S_assignable<_U1, _U2>())
>+	{
>+	  first = std::forward<_U1>(__p.first);
>+	  second = std::forward<_U2>(__p.second);
>+	  return *this;
>+	}
>+#else
>+      // C++11/14/17 implementation using enable_if, partially constexpr.
> 
>       /** The default constructor creates @c first and @c second using their
>        *  respective default constructors.  */
>@@ -281,9 +462,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	explicit constexpr pair(const pair<_U1, _U2>& __p)
> 	: first(__p.first), second(__p.second) { }
> 
>-      constexpr pair(const pair&) = default;	///< Copy constructor
>-      constexpr pair(pair&&) = default;		///< Move constructor
>-
> #if _GLIBCXX_USE_DEPRECATED
>     private:
>       /// @cond undocumented
>@@ -341,7 +519,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        _GLIBCXX_DEPRECATED_SUGGEST("nullptr")
>        explicit pair(__null_ptr_constant, _U2&& __y)
>        : first(nullptr), second(std::forward<_U2>(__y)) { }
>-#endif // _GLIBCXX_USE_DEPRECATED
>+#endif
> 
>       template<typename _U1, typename _U2, typename
> 	       enable_if<_PCCP::template
>@@ -382,10 +560,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	: first(std::forward<_U1>(__p.first)),
> 	  second(std::forward<_U2>(__p.second)) { }
> 
>-      template<typename... _Args1, typename... _Args2>
>-	_GLIBCXX20_CONSTEXPR
>-        pair(piecewise_construct_t, tuple<_Args1...>, tuple<_Args2...>);
>-
>       _GLIBCXX20_CONSTEXPR pair&
>       operator=(typename conditional<
> 		__and_<is_copy_assignable<_T1>,
>@@ -433,24 +607,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	  second = std::forward<_U2>(__p.second);
> 	  return *this;
> 	}
>-
>-      /// Swap the first members and then the second members.
>-      _GLIBCXX20_CONSTEXPR void
>-      swap(pair& __p)
>-      noexcept(__and_<__is_nothrow_swappable<_T1>,
>-                      __is_nothrow_swappable<_T2>>::value)
>-      {
>-	using std::swap;
>-	swap(first, __p.first);
>-	swap(second, __p.second);
>-      }
>-
>-    private:
>-      template<typename... _Args1, size_t... _Indexes1,
>-	       typename... _Args2, size_t... _Indexes2>
>-	_GLIBCXX20_CONSTEXPR
>-        pair(tuple<_Args1...>&, tuple<_Args2...>&,
>-	     _Index_tuple<_Indexes1...>, _Index_tuple<_Indexes2...>);
>+#endif // lib concepts
> #else
>       // C++03 implementation
> 
>diff --git a/libstdc++-v3/testsuite/20_util/pair/cons/99957.cc b/libstdc++-v3/testsuite/20_util/pair/cons/99957.cc
>index d300292b463..a3e6a452583 100644
>--- a/libstdc++-v3/testsuite/20_util/pair/cons/99957.cc
>+++ b/libstdc++-v3/testsuite/20_util/pair/cons/99957.cc
>@@ -16,7 +16,7 @@
> // <http://www.gnu.org/licenses/>.
> 
> // { dg-options "-Wdeprecated" }
>-// { dg-do compile { target c++11 } }
>+// { dg-do compile { target { c++11 && { ! c++20 } } } }
> 
> #include <utility>
> 
>diff --git a/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc b/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc
>index 508ca32ecb7..ecd5acf9375 100644
>--- a/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc
>+++ b/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc
>@@ -83,12 +83,12 @@ void f7(std::pair<long, long>) {}
> 
> std::pair<ExplicitDefault, int> f8()
> {
>-  return {}; // { dg-error "could not convert" }
>+  return {}; // { dg-error "convert" }
> }
> 
> std::pair<ExplicitDefaultDefault, int> f9()
> {
>-  return {}; // { dg-error "could not convert" }
>+  return {}; // { dg-error "convert" }
> }
> 
> void f10(std::pair<ExplicitDefault, int>) {}
>@@ -107,8 +107,8 @@ void test_arg_passing()
>   f7({1,2});
>   f7(std::pair<int, int>{});
>   f7(std::pair<long, long>{});
>-  f10({}); // { dg-error "could not convert" }
>-  f11({}); // { dg-error "could not convert" }
>+  f10({}); // { dg-error "convert" }
>+  f11({}); // { dg-error "convert" }
>   f10(std::pair<ExplicitDefault, int>{});
>   f11(std::pair<ExplicitDefaultDefault, int>{});
> }



More information about the Libstdc++ mailing list