libstdc++: Deprecate non-standard std::pair constructors [PR 99957]
authorJonathan Wakely <jwakely@redhat.com>
Wed, 28 Apr 2021 16:46:01 +0000 (17:46 +0100)
committerJonathan Wakely <jwakely@redhat.com>
Wed, 28 Apr 2021 16:56:50 +0000 (17:56 +0100)
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.

libstdc++-v3/include/bits/stl_pair.h
libstdc++-v3/testsuite/20_util/pair/40925.cc
libstdc++-v3/testsuite/20_util/pair/cons/99957.cc [new file with mode: 0644]
libstdc++-v3/testsuite/20_util/pair/cons/explicit_construct.cc

index 70262f9..883d744 100644 (file)
@@ -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
     };
 
index 157bef6..c95cb38 100644 (file)
@@ -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 (file)
index 0000000..d75ff21
--- /dev/null
@@ -0,0 +1,28 @@
+// { 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" }
index 3d75e6d..508ca32 100644 (file)
@@ -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" }