[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