[libcxx-commits] [libcxx] 1e02158 - [libc++] Implement LWG3843 (std::expected<T, E>::value() & assumes E is copy constructible)

via libcxx-commits libcxx-commits at lists.llvm.org
Fri Jul 7 20:57:22 PDT 2023


Author: yrong
Date: 2023-07-08T11:57:00+08:00
New Revision: 1e02158666e0be2b1b5facf07842f9c15bf347b1

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

LOG: [libc++] Implement LWG3843 (std::expected<T,E>::value() & assumes E is copy constructible)

Implement LWG3843 (std::expected<T,E>::value() & assumes E is copy constructible)
https://wg21.link/LWG3843

Reviewed By: #libc, ldionne

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

Added: 
    libcxx/test/libcxx/utilities/expected/expected.expected/value.observers.verify.cpp

Modified: 
    libcxx/docs/Status/Cxx23Issues.csv
    libcxx/include/__expected/expected.h
    libcxx/test/std/utilities/expected/expected.expected/observers/value.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index 6cfff4dc420c24..aa7b69b569b572 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -291,7 +291,7 @@
 "`3828 <https://wg21.link/LWG3828>`__","Sync ``intmax_t`` and ``uintmax_t`` with C2x","February 2023","","",""
 "`3833 <https://wg21.link/LWG3833>`__","Remove specialization ``template<size_t N> struct formatter<const charT[N], charT>``","February 2023","|Complete|","17.0","|format|"
 "`3836 <https://wg21.link/LWG3836>`__","``std::expected<bool, E1>`` conversion constructor ``expected(const expected<U, G>&)`` should take precedence over ``expected(U&&)`` with operator ``bool``","February 2023","","",""
-"`3843 <https://wg21.link/LWG3843>`__","``std::expected<T,E>::value() &`` assumes ``E`` is copy constructible","February 2023","","",""
+"`3843 <https://wg21.link/LWG3843>`__","``std::expected<T,E>::value() &`` assumes ``E`` is copy constructible","February 2023","|Complete|","17.0",""
 "`3847 <https://wg21.link/LWG3847>`__","``ranges::to`` can still return views","February 2023","","","|ranges|"
 "`3862 <https://wg21.link/LWG3862>`__","``basic_const_iterator``'s ``common_type`` specialization is underconstrained","February 2023","","",""
 "`3865 <https://wg21.link/LWG3865>`__","Sorting a range of ``pairs``","February 2023","|Complete|","17.0","|ranges|"

diff  --git a/libcxx/include/__expected/expected.h b/libcxx/include/__expected/expected.h
index 3c6e592604b2f2..edcffc8b4579f9 100644
--- a/libcxx/include/__expected/expected.h
+++ b/libcxx/include/__expected/expected.h
@@ -46,6 +46,7 @@
 #include <__type_traits/negation.h>
 #include <__type_traits/remove_cv.h>
 #include <__type_traits/remove_cvref.h>
+#include <__utility/as_const.h>
 #include <__utility/exception_guard.h>
 #include <__utility/forward.h>
 #include <__utility/in_place.h>
@@ -559,29 +560,35 @@ class expected {
   _LIBCPP_HIDE_FROM_ABI constexpr bool has_value() const noexcept { return __has_val_; }
 
   _LIBCPP_HIDE_FROM_ABI constexpr const _Tp& value() const& {
+    static_assert(is_copy_constructible_v<_Err>, "error_type has to be copy constructible");
     if (!__has_val_) {
-      std::__throw_bad_expected_access<_Err>(__union_.__unex_);
+      std::__throw_bad_expected_access<_Err>(std::as_const(error()));
     }
     return __union_.__val_;
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr _Tp& value() & {
+    static_assert(is_copy_constructible_v<_Err>, "error_type has to be copy constructible");
     if (!__has_val_) {
-      std::__throw_bad_expected_access<_Err>(__union_.__unex_);
+      std::__throw_bad_expected_access<_Err>(std::as_const(error()));
     }
     return __union_.__val_;
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr const _Tp&& value() const&& {
+    static_assert(is_copy_constructible_v<_Err> && is_constructible_v<_Err, decltype(std::move(error()))>,
+                  "error_type has to be both copy constructible and constructible from decltype(std::move(error()))");
     if (!__has_val_) {
-      std::__throw_bad_expected_access<_Err>(std::move(__union_.__unex_));
+      std::__throw_bad_expected_access<_Err>(std::move(error()));
     }
     return std::move(__union_.__val_);
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr _Tp&& value() && {
+    static_assert(is_copy_constructible_v<_Err> && is_constructible_v<_Err, decltype(std::move(error()))>,
+                  "error_type has to be both copy constructible and constructible from decltype(std::move(error()))");
     if (!__has_val_) {
-      std::__throw_bad_expected_access<_Err>(std::move(__union_.__unex_));
+      std::__throw_bad_expected_access<_Err>(std::move(error()));
     }
     return std::move(__union_.__val_);
   }

diff  --git a/libcxx/test/libcxx/utilities/expected/expected.expected/value.observers.verify.cpp b/libcxx/test/libcxx/utilities/expected/expected.expected/value.observers.verify.cpp
new file mode 100644
index 00000000000000..b70f9ac6e02d40
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/expected/expected.expected/value.observers.verify.cpp
@@ -0,0 +1,131 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
+// Test the mandates
+// constexpr T& value() & ;
+// constexpr const T& value() const &;
+// Mandates: is_copy_constructible_v<E> is true.
+
+// constexpr T&& value() &&;
+// constexpr const T&& value() const &&;
+// Mandates: is_copy_constructible_v<E> is true and is_constructible_v<E, decltype(std::move(error()))> is true.
+
+#include <expected>
+#include <utility>
+
+#include "MoveOnly.h"
+
+struct CopyConstructible {
+  constexpr CopyConstructible()                         = default;
+  constexpr CopyConstructible(const CopyConstructible&) = default;
+};
+
+struct CopyConstructibleButNotMoveConstructible {
+  constexpr CopyConstructibleButNotMoveConstructible()                                                 = default;
+  constexpr CopyConstructibleButNotMoveConstructible(const CopyConstructibleButNotMoveConstructible&)  = default;
+  constexpr CopyConstructibleButNotMoveConstructible(CopyConstructibleButNotMoveConstructible&&)       = delete;
+  constexpr CopyConstructibleButNotMoveConstructible(const CopyConstructibleButNotMoveConstructible&&) = delete;
+};
+
+struct CopyConstructibleAndMoveConstructible {
+  constexpr CopyConstructibleAndMoveConstructible()                                             = default;
+  constexpr CopyConstructibleAndMoveConstructible(const CopyConstructibleAndMoveConstructible&) = default;
+  constexpr CopyConstructibleAndMoveConstructible(CopyConstructibleAndMoveConstructible&&)      = default;
+};
+
+// clang-format off
+void test() {
+
+  // Test & overload
+  {
+    // is_copy_constructible_v<E> is true.
+    {
+      std::expected<int, CopyConstructible> e;
+      [[maybe_unused]] auto val = e.value();
+    }
+
+    // is_copy_constructible_v<E> is false.
+    {
+      std::expected<int, MoveOnly> e;
+      [[maybe_unused]] auto val = e.value();
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}error_type has to be copy constructible}}
+    }
+  }
+
+  // Test const& overload
+  {
+    // is_copy_constructible_v<E> is true.
+    {
+      const std::expected<int, CopyConstructible> e;
+      [[maybe_unused]] auto val = e.value();
+    }
+
+    // is_copy_constructible_v<E> is false.
+    {
+      const std::expected<int, MoveOnly> e;
+      [[maybe_unused]] auto val = e.value();
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}error_type has to be copy constructible}}
+    }
+  }
+
+  // Test && overload
+  {
+    // is_copy_constructible_v<E> is false.
+    {
+      std::expected<int, MoveOnly> e;
+      [[maybe_unused]] auto val = std::move(e).value();
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}error_type has to be both copy constructible and constructible from decltype(std::move(error()))}}
+    }
+
+    // is_copy_constructible_v<E> is true and is_constructible_v<E, decltype(std::move(error()))> is true.
+    {
+      std::expected<int, CopyConstructibleAndMoveConstructible> e;
+      [[maybe_unused]] auto val = std::move(e).value();
+    }
+
+    // is_copy_constructible_v<E> is true and is_constructible_v<E, decltype(std::move(error()))> is false.
+    {
+      std::expected<int, CopyConstructibleButNotMoveConstructible> e;
+      [[maybe_unused]] auto val = std::move(e).value();
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}error_type has to be both copy constructible and constructible from decltype(std::move(error()))}}
+    }
+  }
+
+  // Test const&& overload
+  {
+    // is_copy_constructible_v<E> is false.
+    {
+      const std::expected<int, MoveOnly> e;
+      [[maybe_unused]] auto val = std::move(e).value();
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}error_type has to be both copy constructible and constructible from decltype(std::move(error()))}}
+    }
+
+    // is_copy_constructible_v<E> is true and is_constructible_v<E, decltype(std::move(error()))> is true.
+    {
+      const std::expected<int, CopyConstructibleAndMoveConstructible> e;
+      [[maybe_unused]] auto val = std::move(e).value();
+    }
+
+    // is_copy_constructible_v<E> is true and is_constructible_v<E, decltype(std::move(error()))> is false.
+    {
+      const std::expected<int, CopyConstructibleButNotMoveConstructible> e;
+      [[maybe_unused]] auto val = std::move(e).value();
+      // expected-error-re@*:* {{{{(static_assert|static assertion)}} failed {{.*}}error_type has to be both copy constructible and constructible from decltype(std::move(error()))}}
+    }
+  }
+// These diagnostics happen when we try to construct bad_expected_access from the non copy-constructible error type.
+#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
+  // expected-error-re@*:* {{call to deleted constructor of{{.*}}}}
+  // expected-error-re@*:* {{call to deleted constructor of{{.*}}}}
+  // expected-error-re@*:* {{call to deleted constructor of{{.*}}}}
+  // expected-error-re@*:* {{call to deleted constructor of{{.*}}}}
+#endif
+}
+// clang-format on

diff  --git a/libcxx/test/std/utilities/expected/expected.expected/observers/value.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/observers/value.pass.cpp
index 718c307bed6dab..44d1b2fc2f7c39 100644
--- a/libcxx/test/std/utilities/expected/expected.expected/observers/value.pass.cpp
+++ b/libcxx/test/std/utilities/expected/expected.expected/observers/value.pass.cpp
@@ -18,7 +18,6 @@
 #include <type_traits>
 #include <utility>
 
-#include "MoveOnly.h"
 #include "test_macros.h"
 
 constexpr bool test() {
@@ -75,17 +74,32 @@ void testException() {
     }
   }
 
-  // MoveOnly
+#endif // TEST_HAS_NO_EXCEPTIONS
+}
+
+void testAsConst() {
+#ifndef TEST_HAS_NO_EXCEPTIONS
+  struct Error {
+    enum { Default, MutableRefCalled, ConstRefCalled } From = Default;
+    Error()                                                 = default;
+    Error(const Error&) { From = ConstRefCalled; }
+    Error(Error&) { From = MutableRefCalled; }
+    Error(Error&& e) { From = e.From; }
+  };
+
+  // Test & overload
   {
-    std::expected<int, MoveOnly> e(std::unexpect, 5);
+    std::expected<int, Error> e(std::unexpect, Error());
     try {
-      (void) std::move(e).value();
+      (void)e.value();
       assert(false);
-    } catch (const std::bad_expected_access<MoveOnly>& ex) {
-      assert(ex.error() == 5);
+    } catch (const std::bad_expected_access<Error>& ex) {
+      assert(ex.error().From == Error::ConstRefCalled);
     }
   }
 
+  // There are no effects for `const &` overload.
+
 #endif // TEST_HAS_NO_EXCEPTIONS
 }
 
@@ -93,5 +107,7 @@ int main(int, char**) {
   test();
   static_assert(test());
   testException();
+  testAsConst();
+
   return 0;
 }


        


More information about the libcxx-commits mailing list