[RFC] Deprecate non-standard constructors in std::pair
Jonathan Wakely
jwakely@redhat.com
Wed Apr 28 16:57:35 GMT 2021
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.
>
> libstdc++: Deprecate non-standard std::pair constructors [PR 99957]
>
> This deprecates the non-standard std::pair constructors that support
> construction from an rvalue and a literal zero used as a null pointer
> constant. We can't just add the deprecated attribute to those
> constructors, because they're currently used by correct code when they
> are a better match than the constructors required by the standard e.g.
>
> int i = 0;
> const int j = 0;
> std::pair<int, int> p(i, j); // uses pair(U1&&, const int&)
>
> This patch adjusts the parameter types and constraints of those
> constructors so that they only get used for literal zeros, and the
> pair(U1&&, U2&&) constructor gets used otherwise. Once they're only used
> for initializations that should be ill-formed we can add the deprecated
> attribute.
>
> The deprecated attribute is used to suggest that the user code uses
> nullptr, which avoids the problem of 0 deducing as int instead of a null
> pointer constant.
I've pushed this to trunk, after testing on powerpc64le-linux.
>commit e20794c814c5961f0f33381a8eb3dff4fc741b5a
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date: Wed Apr 7 17:20:43 2021
>
> libstdc++: Deprecate non-standard std::pair constructors [PR 99957]
>
> This deprecates the non-standard std::pair constructors that support
> construction from an rvalue and a literal zero used as a null pointer
> constant. We can't just add the deprecated attribute to those
> constructors, because they're currently used by correct code when they
> are a better match than the constructors required by the standard e.g.
>
> int i = 0;
> const int j = 0;
> std::pair<int, int> p(i, j); // uses pair(U1&&, const int&)
>
> This patch adjusts the parameter types and constraints of those
> constructors so that they only get used for literal zeros, and the
> pair(U1&&, U2&&) constructor gets used otherwise. Once they're only used
> for initializations that should be ill-formed we can add the deprecated
> attribute.
>
> The deprecated attribute is used to suggest that the user code uses
> nullptr, which avoids the problem of 0 deducing as int instead of a null
> pointer constant.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/99957
> * include/bits/stl_pair.h (_PCC::_MoveCopyPair, _PCC::_CopyMovePair):
> Combine and replace with ...
> (_PCC::_DeprConsPair): New SFINAE helper function.
> (pair): Merge preprocessor blocks so that all C++03 members
> are defined together at the end.
> (pair::pair(const _T1&, _U2&&), pair::pair(_U1&&, const _T2&)):
> Replace _T1 and _T2 parameters with __null_ptr_constant and
> adjust constraints.
> * testsuite/20_util/pair/40925.cc: Use nullptr instead of 0.
> * testsuite/20_util/pair/cons/explicit_construct.cc: Likewise.
> * testsuite/20_util/pair/cons/99957.cc: New test.
>
>diff --git a/libstdc++-v3/include/bits/stl_pair.h b/libstdc++-v3/include/bits/stl_pair.h
>index 70262f9508f..883d7441b3d 100644
>--- a/libstdc++-v3/include/bits/stl_pair.h
>+++ b/libstdc++-v3/include/bits/stl_pair.h
>@@ -128,34 +128,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> is_convertible<_U2&&, _T2>>::value;
> }
>
>- template <bool __implicit, typename _U1, typename _U2>
>- static constexpr bool _CopyMovePair()
>- {
>- using __do_converts = __and_<is_convertible<const _U1&, _T1>,
>- is_convertible<_U2&&, _T2>>;
>- using __converts = typename conditional<__implicit,
>- __do_converts,
>- __not_<__do_converts>>::type;
>- return __and_<is_constructible<_T1, const _U1&>,
>- is_constructible<_T2, _U2&&>,
>- __converts
>- >::value;
>- }
>
> template <bool __implicit, typename _U1, typename _U2>
>- static constexpr bool _MoveCopyPair()
>+ static constexpr bool _DeprConsPair()
> {
> using __do_converts = __and_<is_convertible<_U1&&, _T1>,
>- is_convertible<const _U2&, _T2>>;
>+ is_convertible<_U2&&, _T2>>;
> using __converts = typename conditional<__implicit,
>- __do_converts,
>- __not_<__do_converts>>::type;
>+ __do_converts,
>+ __not_<__do_converts>>::type;
> return __and_<is_constructible<_T1, _U1&&>,
>- is_constructible<_T2, const _U2&&>,
>+ is_constructible<_T2, _U2&&>,
> __converts
>- >::value;
>+ >::value;
> }
>- };
>+ };
>
> template <typename _T1, typename _T2>
> struct _PCC<false, _T1, _T2>
>@@ -183,7 +170,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> {
> return false;
> }
>- };
>+ };
> #endif // C++11
>
> template<typename _U1, typename _U2> class __pair_base
>@@ -217,22 +204,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> _T1 first; ///< The first member
> _T2 second; ///< The second member
>
>- // _GLIBCXX_RESOLVE_LIB_DEFECTS
>- // 265. std::pair::pair() effects overly restrictive
>+#if __cplusplus >= 201103L
>+ // C++11 (and later) implementation.
>+
> /** The default constructor creates @c first and @c second using their
> * respective default constructors. */
>-#if __cplusplus >= 201103L
> template <typename _U1 = _T1,
> typename _U2 = _T2,
> typename enable_if<__and_<
> __is_implicitly_default_constructible<_U1>,
> __is_implicitly_default_constructible<_U2>>
> ::value, bool>::type = true>
>-#endif
>- _GLIBCXX_CONSTEXPR pair()
>+ constexpr pair()
> : first(), second() { }
>
>-#if __cplusplus >= 201103L
> template <typename _U1 = _T1,
> typename _U2 = _T2,
> typename enable_if<__and_<
>@@ -244,13 +229,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ::value, bool>::type = false>
> explicit constexpr pair()
> : first(), second() { }
>-#endif
>
>-#if __cplusplus < 201103L
>- /// Two objects may be passed to a @c pair constructor to be copied.
>- pair(const _T1& __a, const _T2& __b)
>- : first(__a), second(__b) { }
>-#else
> // Shortcut for constraining the templates that don't take pairs.
> /// @cond undocumented
> using _PCCP = _PCC<true, _T1, _T2>;
>@@ -275,14 +254,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> bool>::type=false>
> explicit constexpr pair(const _T1& __a, const _T2& __b)
> : first(__a), second(__b) { }
>-#endif
>
>-#if __cplusplus < 201103L
>- /// There is also a templated constructor to convert from other pairs.
>- template<typename _U1, typename _U2>
>- pair(const pair<_U1, _U2>& __p)
>- : first(__p.first), second(__p.second) { }
>-#else
> // Shortcut for constraining the templates that take pairs.
> /// @cond undocumented
> template <typename _U1, typename _U2>
>@@ -308,40 +280,68 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> bool>::type=false>
> explicit constexpr pair(const pair<_U1, _U2>& __p)
> : first(__p.first), second(__p.second) { }
>-#endif
>
>-#if __cplusplus >= 201103L
> constexpr pair(const pair&) = default; ///< Copy constructor
> constexpr pair(pair&&) = default; ///< Move constructor
>
>- // DR 811.
>- template<typename _U1, typename
>- enable_if<_PCCP::template
>- _MoveCopyPair<true, _U1, _T2>(),
>- bool>::type=true>
>- constexpr pair(_U1&& __x, const _T2& __y)
>- : first(std::forward<_U1>(__x)), second(__y) { }
>+#if _GLIBCXX_USE_DEPRECATED
>+ private:
>+ /// @cond undocumented
>
>- template<typename _U1, typename
>- enable_if<_PCCP::template
>- _MoveCopyPair<false, _U1, _T2>(),
>- bool>::type=false>
>- explicit constexpr pair(_U1&& __x, const _T2& __y)
>- : first(std::forward<_U1>(__x)), second(__y) { }
>+ // A type which can be constructed from literal zero, but not nullptr
>+ struct __null_ptr_constant
>+ {
>+ __null_ptr_constant(int __null_ptr_constant::*) { }
>+ template<typename _Tp,
>+ typename = __enable_if_t<is_null_pointer<_Tp>::value>>
>+ __null_ptr_constant(_Tp) = delete;
>+ };
>
>- template<typename _U2, typename
>- enable_if<_PCCP::template
>- _CopyMovePair<true, _T1, _U2>(),
>- bool>::type=true>
>- constexpr pair(const _T1& __x, _U2&& __y)
>- : first(__x), second(std::forward<_U2>(__y)) { }
>+ // True if type _Up is one of _Tp& or const _Tp&
>+ template<typename _Up, typename _Tp>
>+ using __is_lvalue_of
>+ = __or_<is_same<_Up, const _Tp&>, is_same<_Up, _Tp&>>;
>
>- template<typename _U2, typename
>- enable_if<_PCCP::template
>- _CopyMovePair<false, _T1, _U2>(),
>- bool>::type=false>
>- explicit pair(const _T1& __x, _U2&& __y)
>- : first(__x), second(std::forward<_U2>(__y)) { }
>+ /// @endcond
>+ public:
>+
>+ // Deprecated extensions to DR 811.
>+ template<typename _U1,
>+ __enable_if_t<!__is_lvalue_of<_U1, _T1>::value
>+ && _PCCP::template
>+ _DeprConsPair<true, _U1, nullptr_t>(),
>+ bool> = true>
>+ _GLIBCXX_DEPRECATED_SUGGEST("nullptr")
>+ constexpr pair(_U1&& __x, __null_ptr_constant)
>+ : first(std::forward<_U1>(__x)), second(nullptr) { }
>+
>+ template<typename _U1,
>+ __enable_if_t<!__is_lvalue_of<_U1, _T1>::value
>+ && _PCCP::template
>+ _DeprConsPair<false, _U1, nullptr_t>(),
>+ bool> = false>
>+ _GLIBCXX_DEPRECATED_SUGGEST("nullptr")
>+ explicit constexpr pair(_U1&& __x, __null_ptr_constant)
>+ : first(std::forward<_U1>(__x)), second(nullptr) { }
>+
>+ template<typename _U2,
>+ __enable_if_t<!__is_lvalue_of<_U2, _T2>::value
>+ && _PCCP::template
>+ _DeprConsPair<true, nullptr_t, _U2>(),
>+ bool> = true>
>+ _GLIBCXX_DEPRECATED_SUGGEST("nullptr")
>+ constexpr pair(__null_ptr_constant, _U2&& __y)
>+ : first(nullptr), second(std::forward<_U2>(__y)) { }
>+
>+ template<typename _U2,
>+ __enable_if_t<!__is_lvalue_of<_U2, _T2>::value
>+ && _PCCP::template
>+ _DeprConsPair<false, nullptr_t, _U2>(),
>+ bool> = false>
>+ _GLIBCXX_DEPRECATED_SUGGEST("nullptr")
>+ explicit pair(__null_ptr_constant, _U2&& __y)
>+ : first(nullptr), second(std::forward<_U2>(__y)) { }
>+#endif // _GLIBCXX_USE_DEPRECATED
>
> template<typename _U1, typename _U2, typename
> enable_if<_PCCP::template
>@@ -451,6 +451,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> _GLIBCXX20_CONSTEXPR
> pair(tuple<_Args1...>&, tuple<_Args2...>&,
> _Index_tuple<_Indexes1...>, _Index_tuple<_Indexes2...>);
>+#else
>+ // C++03 implementation
>+
>+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
>+ // 265. std::pair::pair() effects overly restrictive
>+ /** The default constructor creates @c first and @c second using their
>+ * respective default constructors. */
>+ pair() : first(), second() { }
>+
>+ /// Two objects may be passed to a `pair` constructor to be copied.
>+ pair(const _T1& __a, const _T2& __b)
>+ : first(__a), second(__b) { }
>+
>+ /// Templated constructor to convert from other pairs.
>+ template<typename _U1, typename _U2>
>+ pair(const pair<_U1, _U2>& __p)
>+ : first(__p.first), second(__p.second) { }
> #endif // C++11
> };
>
>diff --git a/libstdc++-v3/testsuite/20_util/pair/40925.cc b/libstdc++-v3/testsuite/20_util/pair/40925.cc
>index 157bef621be..c95cb380b18 100644
>--- a/libstdc++-v3/testsuite/20_util/pair/40925.cc
>+++ b/libstdc++-v3/testsuite/20_util/pair/40925.cc
>@@ -20,7 +20,7 @@
> #include <utility>
>
> struct X
>-{
>+{
> explicit X(int, int) { }
>
> private:
>@@ -36,7 +36,7 @@ private:
> move_only(const move_only&) = delete;
> };
>
>-// libstdc++/40925
>+// libstdc++/40925 and LWG 811
> void test01()
> {
> int *ip = 0;
>@@ -52,10 +52,12 @@ void test01()
> std::pair<int X::*, int X::*> p7(0, mp);
> std::pair<int X::*, int X::*> p8(mp, mp);
>
>- std::pair<int*, move_only> p9(0, move_only());
>- std::pair<int X::*, move_only> p10(0, move_only());
>- std::pair<move_only, int*> p11(move_only(), 0);
>- std::pair<move_only, int X::*> p12(move_only(), 0);
>+ // LWG 811 resolution doesn't support move-only types,
>+ // so we have to use nullptr here not a literal 0.
>+ std::pair<int*, move_only> p9(nullptr, move_only());
>+ std::pair<int X::*, move_only> p10(nullptr, move_only());
>+ std::pair<move_only, int*> p11(move_only(), nullptr);
>+ std::pair<move_only, int X::*> p12(move_only(), nullptr);
>
> std::pair<int*, move_only> p13(ip, move_only());
> std::pair<int X::*, move_only> p14(mp, move_only());
>diff --git a/libstdc++-v3/testsuite/20_util/pair/cons/99957.cc b/libstdc++-v3/testsuite/20_util/pair/cons/99957.cc
>new file mode 100644
>index 00000000000..b3114131a9d
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/pair/cons/99957.cc
>@@ -0,0 +1,45 @@
>+// Copyright (C) 2021 Free Software Foundation, Inc.
>+//
>+// This file is part of the GNU ISO C++ Library. This library is free
>+// software; you can redistribute it and/or modify it under the
>+// terms of the GNU General Public License as published by the
>+// Free Software Foundation; either version 3, or (at your option)
>+// any later version.
>+
>+// This library is distributed in the hope that it will be useful,
>+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+// GNU General Public License for more details.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3. If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-options "-Wdeprecated" }
>+// { dg-do compile { target { c++11 } } }
>+
>+#include <utility>
>+
>+using std::pair;
>+
>+struct MoveOnly
>+{
>+ MoveOnly() = default;
>+ MoveOnly(MoveOnly&&) {}
>+};
>+
>+struct ExplicitMoveOnly
>+{
>+ ExplicitMoveOnly() = default;
>+ ExplicitMoveOnly(ExplicitMoveOnly&&) {}
>+ explicit ExplicitMoveOnly(MoveOnly&&) {}
>+};
>+
>+// PR libstdc++/99957
>+// check non-standard constructors are deprecated
>+
>+pair<int*, ExplicitMoveOnly> v14{0, MoveOnly{}}; // { dg-warning "deprecated" }
>+pair<ExplicitMoveOnly, int*> v15{MoveOnly{}, 0}; // { dg-warning "deprecated" }
>+
>+pair<int*, MoveOnly> v16 = {0, MoveOnly{}}; // { dg-warning "deprecated" }
>+pair<MoveOnly, int*> v17 = {MoveOnly{}, 0}; // { dg-warning "deprecated" }
>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 3d75e6dbb91..508ca32ecb7 100644
>--- a/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc
>+++ b/libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc
>@@ -126,10 +126,10 @@ struct ExplicitMoveOnly
> explicit ExplicitMoveOnly(MoveOnly&&) {}
> };
>
>-std::pair<int*, ExplicitMoveOnly> v14{0, MoveOnly{}};
>-std::pair<ExplicitMoveOnly, int*> v15{MoveOnly{}, 0};
>+std::pair<int*, ExplicitMoveOnly> v14{nullptr, MoveOnly{}};
>+std::pair<ExplicitMoveOnly, int*> v15{MoveOnly{}, nullptr};
>
> std::pair<int*, ExplicitMoveOnly> v16 =
>- {0, MoveOnly{}}; // { dg-error "could not convert" }
>+ {nullptr, MoveOnly{}}; // { dg-error "could not convert" }
> std::pair<ExplicitMoveOnly, int*> v17 =
>- {MoveOnly{}, 0}; // { dg-error "could not convert" }
>+ {MoveOnly{}, nullptr}; // { dg-error "could not convert" }
More information about the Libstdc++
mailing list