[libcxx-commits] [libcxx] 1e628d0 - [libc++] Do not enable P1951 before C++23, since it's a breaking change

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Mon Sep 27 14:06:53 PDT 2021


Author: Louis Dionne
Date: 2021-09-27T17:06:44-04:00
New Revision: 1e628d0c1405ad6fd1a8036cdef19692dbf8ac4b

URL: https://github.com/llvm/llvm-project/commit/1e628d0c1405ad6fd1a8036cdef19692dbf8ac4b
DIFF: https://github.com/llvm/llvm-project/commit/1e628d0c1405ad6fd1a8036cdef19692dbf8ac4b.diff

LOG: [libc++] Do not enable P1951 before C++23, since it's a breaking change

In reaction to the issues raised by Richard in https://llvm.org/D109066,
this commit does not apply P1951 as a DR in previous standard modes,
since it breaks valid code.

I do believe it should be applied as a DR, however ideally we'd get some
sort of statement from the Committee to this effect (and all implementations
would behave consistently). In the meantime, only implement P1951 starting
with C++23 -- we can always come back and apply it as a DR if that's what
the Committee says.

Differential Revision: https://reviews.llvm.org/D110347

Added: 
    libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.brace-init.P1951.pass.cpp

Modified: 
    libcxx/include/__utility/pair.h
    libcxx/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__utility/pair.h b/libcxx/include/__utility/pair.h
index 93cbc9013c698..959a8db5b3ecc 100644
--- a/libcxx/include/__utility/pair.h
+++ b/libcxx/include/__utility/pair.h
@@ -168,18 +168,28 @@ struct _LIBCPP_TEMPLATE_VIS pair
                    is_nothrow_copy_constructible<second_type>::value)
         : first(__t1), second(__t2) {}
 
-    template<class _U1 = _T1, class _U2 = _T2, typename enable_if<
-             _CheckArgs::template __enable_explicit<_U1, _U2>()
-    >::type* = nullptr>
+    template <
+#if _LIBCPP_STD_VER > 20 // http://wg21.link/P1951
+        class _U1 = _T1, class _U2 = _T2,
+#else
+        class _U1, class _U2,
+#endif
+        typename enable_if<_CheckArgs::template __enable_explicit<_U1, _U2>()>::type* = nullptr
+    >
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
     explicit pair(_U1&& __u1, _U2&& __u2)
         _NOEXCEPT_((is_nothrow_constructible<first_type, _U1>::value &&
                     is_nothrow_constructible<second_type, _U2>::value))
         : first(_VSTD::forward<_U1>(__u1)), second(_VSTD::forward<_U2>(__u2)) {}
 
-    template<class _U1 = _T1, class _U2 = _T2, typename enable_if<
-            _CheckArgs::template __enable_implicit<_U1, _U2>()
-    >::type* = nullptr>
+    template <
+#if _LIBCPP_STD_VER > 20 // http://wg21.link/P1951
+        class _U1 = _T1, class _U2 = _T2,
+#else
+        class _U1, class _U2,
+#endif
+        typename enable_if<_CheckArgs::template __enable_implicit<_U1, _U2>()>::type* = nullptr
+    >
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
     pair(_U1&& __u1, _U2&& __u2)
         _NOEXCEPT_((is_nothrow_constructible<first_type, _U1>::value &&

diff  --git a/libcxx/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp b/libcxx/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp
index 5086a298ac037..33b5711e22183 100644
--- a/libcxx/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp
+++ b/libcxx/test/std/utilities/utility/pairs/pairs.pair/U_V.pass.cpp
@@ -98,14 +98,13 @@ int main(int, char**)
 #endif
 
     // Test support for http://wg21.link/P1951, default arguments for pair's constructor.
-    // Basically, this turns copies for brace initialization into moves. Note that libc++
-    // applies this in all standard modes.
-#if TEST_STD_VER > 20 || defined(_LIBCPP_VERSION)
+    // Basically, this turns copies for brace initialization into moves.
+#if TEST_STD_VER > 20
     {
         struct TrackInit {
             TrackInit() = default;
-            constexpr TrackInit(TrackInit const&) : wasCopyInit(true) { }
-            constexpr TrackInit(TrackInit&&) : wasMoveInit(true) { }
+            constexpr TrackInit(TrackInit const& other) : wasMoveInit(other.wasMoveInit), wasCopyInit(true) { }
+            constexpr TrackInit(TrackInit&& other) : wasMoveInit(true), wasCopyInit(other.wasCopyInit) { }
             bool wasMoveInit = false;
             bool wasCopyInit = false;
         };
@@ -138,7 +137,7 @@ int main(int, char**)
             }
         }
     }
-#endif
+#endif // TEST_STD_VER > 20
 
     return 0;
 }

diff  --git a/libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.brace-init.P1951.pass.cpp b/libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.brace-init.P1951.pass.cpp
new file mode 100644
index 0000000000000..aeb7e4516dc9b
--- /dev/null
+++ b/libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.brace-init.P1951.pass.cpp
@@ -0,0 +1,48 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: c++11 || c++14 || c++17 || c++20
+
+// This test makes sure that we don't apply P1951 before C++23, since that is
+// a breaking change. The examples in this test are taken from Richard Smith's
+// comments on https://llvm.org/D109066.
+
+#include <cassert>
+#include <utility>
+#include <vector>
+
+struct A {
+    int *p_;
+    A(int *p) : p_(p) { *p_ += 1; }
+    A(const A& a) : p_(a.p_) { *p_ += 1; }
+    ~A() { *p_ -= 1; }
+};
+
+int main(int, char**) {
+    // Example 1:
+    // Without P1951, we call the `pair(int, const A&)` constructor (the converting constructor is not usable because
+    // we can't deduce from an initializer list), which creates the A temporary as part of the call to f. With P1951,
+    // we call the `pair(U&&, V&&)` constructor, which creates a A temporary inside the pair constructor, and that
+    // temporary doesn't live long enough any more.
+    {
+        int i = 0;
+        auto f = [&](std::pair<std::vector<int>, const A&>) { assert(i >= 1); };
+        f({{42, 43}, &i});
+    }
+
+    // Example 2:
+    // Here, n doesn't need to be captured if we call the `pair(const int&, const long&)` constructor, because
+    // the lvalue-to-rvalue conversion happens in the lambda. But if we call the `pair(U&&, V&&)` constructor
+    // (deducing V = int), then n does need to be captured.
+    {
+        const int n = 5;
+        (void) []{ std::pair<int, long>({1}, n); };
+    }
+
+    return 0;
+}


        


More information about the libcxx-commits mailing list