Merge pull request #146 from steve-downey/lwg4304

Address LWG4304 value_or ill-formed

Make value_or not cause errors when instantiating optional<T&> on
non-returnable T.

Author: steve-downey

include/beman/optional/optional.hpp

@@ -1791,6 +1791,16 @@ class optional<T&> {
      */
     constexpr T& value() const;
 
+    // LWG4304. std::optional<NonReturnable&> is ill-formed due to value_o
+    // Resolution:
+    // -?- Constraints: T is a non-array object type.
+    // -?- Remarks: The return type is unspecified if T is an array type or a
+    //     non-object type. [Note ?: This is to avoid the declaration being
+    //     ill-formed. — end note]
+    //
+    // Implementer Note: Using decay_t as a detail as it is remove_cv_t for
+    // non-array objects, and produces a valid type for arrays and functions,
+    // which are otherwise `required` out.
     /**
      * @brief Returns the contained value if there is one, otherwise returns `u`.
      *
@@ -1799,7 +1809,8 @@ class optional<T&> {
      * @return std::remove_cv_t<T>
      */
     template <class U = std::remove_cv_t<T>>
-    constexpr std::remove_cv_t<T> value_or(U&& u) const;
+        requires(std::is_object_v<T> && !std::is_array_v<T>)
+    constexpr std::decay_t<T> value_or(U&& u) const;
 
     // \ref{optionalref.monadic}, monadic operations
     /**
@@ -1991,7 +2002,8 @@ constexpr T& optional<T&>::value() const {
 
 template <class T>
 template <class U>
-constexpr std::remove_cv_t<T> optional<T&>::value_or(U&& u) const {
+    requires(std::is_object_v<T> && !std::is_array_v<T>)
+constexpr std::decay_t<T> optional<T&>::value_or(U&& u) const {
     static_assert(std::is_constructible_v<std::remove_cv_t<T>, T&>, "T must be constructible from a T&");
     static_assert(std::is_convertible_v<U, std::remove_cv_t<T>>, "Must be able to convert u to T");
     return has_value() ? *value_ : static_cast<std::remove_cv_t<T>>(std::forward<U>(u));

tests/beman/optional/optional_ref.test.cpp

@@ -864,6 +864,29 @@ TEST(OptionalRefTest, OverloadResolutionChecksDangling) {
     //    static_assert(std::is_same_v<decltype(check_dangling("abc")), void>);
 }
 
+namespace {
+int int_func(void) { return 7; }
+} // namespace
+
+TEST(OptionalRefTest, NonReturnableRef) {
+    using IntArray5 = int[5];
+    beman::optional::optional<IntArray5&> o1;
+    IntArray5                             array;
+    beman::optional::optional<IntArray5&> o2{array};
+    EXPECT_FALSE(o1.has_value());
+    EXPECT_TRUE(o2.has_value());
+    // value_or removed for array types
+    // IntArray5 array2;
+    // auto t1 = o1.value_or(array2);
+    // auto t2 = o2.value_or(array2);
+
+    using IntFunc = int(void);
+    beman::optional::optional<IntFunc&> o3;
+    beman::optional::optional<IntFunc&> o4{int_func};
+    EXPECT_FALSE(o3.has_value());
+    EXPECT_TRUE(o4.has_value());
+}
+
 // beman::optional::optional<int const&> foo() {
 //     beman::optional::optional<int> o(10);
 //     return o; // Thanks to a simpler implicit move.