Skip to content

Commit 75532d4

Browse files
dangelogdangelog
committed
optional::transform: ensure that it works with non-movable payloads
This is supposed to work and it's explicitly spelled out in the standard text. If the function called by transform returns a non-movable prvalue, we cannot rely on optional(T&&) constructor for this, because that will not simply materialize it into the payload of the returned optional. Instead, add a new private constructor for optional with a private tag. Inside that constructor, we invoke the function and use it to initialize the payload; this will not require any moves due to guaranteed copy elision. In transform(), we call that constructor, passing the function and the value stored in *this to it. A similar change is needed for the optional<T&>. Since we need to call this private optional<U>'s constructor from a generic optional<T>::transform, we need to grant friendship to all possible optional specializations.
1 parent 1d93a02 commit 75532d4

File tree

3 files changed

+67
-5
lines changed

3 files changed

+67
-5
lines changed

‎include/beman/optional/optional.hpp‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ template <typename T, typename U>
4949
concept optional_ge_rel = requires(const T& t, const U& u) {
5050
{ t >= u } -> std::convertible_to<bool>;
5151
};
52+
53+
struct from_function_t {
54+
explicit from_function_t() = default;
55+
};
56+
57+
inline constexpr from_function_t from_function{};
5258
} // namespace detail
5359

5460
struct in_place_t {
@@ -403,6 +409,13 @@ class optional {
403409
std::destroy_at(std::addressof(value_));
404410
engaged_ = false;
405411
}
412+
413+
template <class U>
414+
friend class optional;
415+
416+
template <class F, class Arg>
417+
constexpr optional(detail::from_function_t, F&& f, Arg&& arg)
418+
: value_(std::invoke(std::forward<F>(f), std::forward<Arg>(arg))), engaged_(true) {}
406419
};
407420

408421
class bad_optional_access : public std::exception {
@@ -785,7 +798,7 @@ constexpr auto optional<T>::transform(F&& f) & {
785798
static_assert(!std::is_same_v<U, in_place_t>);
786799
static_assert(!std::is_same_v<U, nullopt_t>);
787800
static_assert(std::is_object_v<U> || std::is_reference_v<U>); /// References now allowed
788-
return (has_value()) ? optional<U>{std::invoke(std::forward<F>(f), value_)} : optional<U>{};
801+
return (has_value()) ? optional<U>{detail::from_function, std::forward<F>(f), value_} : optional<U>{};
789802
}
790803

791804
template <class T>
@@ -796,7 +809,7 @@ constexpr auto optional<T>::transform(F&& f) && {
796809
static_assert(!std::is_same_v<U, in_place_t>);
797810
static_assert(!std::is_same_v<U, nullopt_t>);
798811
static_assert(std::is_object_v<U> || std::is_reference_v<U>); /// References now allowed
799-
return (has_value()) ? optional<U>{std::invoke(std::forward<F>(f), std::move(value_))} : optional<U>{};
812+
return (has_value()) ? optional<U>{detail::from_function, std::forward<F>(f), std::move(value_)} : optional<U>{};
800813
}
801814

802815
template <class T>
@@ -807,7 +820,7 @@ constexpr auto optional<T>::transform(F&& f) const& {
807820
static_assert(!std::is_same_v<U, in_place_t>);
808821
static_assert(!std::is_same_v<U, nullopt_t>);
809822
static_assert(std::is_object_v<U> || std::is_reference_v<U>); /// References now allowed
810-
return (has_value()) ? optional<U>{std::invoke(std::forward<F>(f), value_)} : optional<U>{};
823+
return (has_value()) ? optional<U>{detail::from_function, std::forward<F>(f), value_} : optional<U>{};
811824
}
812825

813826
template <class T>
@@ -818,7 +831,7 @@ constexpr auto optional<T>::transform(F&& f) const&& {
818831
static_assert(!std::is_same_v<U, in_place_t>);
819832
static_assert(!std::is_same_v<U, nullopt_t>);
820833
static_assert(std::is_object_v<U> || std::is_reference_v<U>); /// References now allowed
821-
return (has_value()) ? optional<U>{std::invoke(std::forward<F>(f), std::move(value_))} : optional<U>{};
834+
return (has_value()) ? optional<U>{detail::from_function, std::forward<F>(f), std::move(value_)} : optional<U>{};
822835
}
823836

824837
/// Calls `f` if the optional is empty
@@ -1207,6 +1220,14 @@ class optional<T&> {
12071220
T& r(std::forward<U>(u));
12081221
value_ = std::addressof(r);
12091222
}
1223+
1224+
template <class U>
1225+
friend class optional;
1226+
1227+
template <class F, class Arg>
1228+
constexpr optional(detail::from_function_t, F&& f, Arg&& arg) {
1229+
convert_ref_init_val(std::invoke(std::forward<F>(f), std::forward<Arg>(arg)));
1230+
}
12101231
};
12111232

12121233
// \rSec3[optionalref.ctor]{Constructors}
@@ -1356,7 +1377,7 @@ constexpr optional<std::invoke_result_t<F, T&>> optional<T&>::transform(F&& f) c
13561377
static_assert((std::is_object_v<U> && !std::is_array_v<U>) || std::is_lvalue_reference_v<U>,
13571378
"Result must be an non-array object or an lvalue reference");
13581379
if (has_value()) {
1359-
return optional<U>{std::invoke(std::forward<F>(f), *value_)};
1380+
return optional<U>{detail::from_function, std::forward<F>(f), *value_};
13601381
} else {
13611382
return optional<U>{};
13621383
}

‎tests/beman/optional/optional_monadic.t.cpp‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
#include <beman/optional/optional.hpp>
55

6+
#include <beman/optional/test_types.hpp>
7+
68
#include <gtest/gtest.h>
79

810
constexpr int get_int(int) { return 42; }
@@ -81,6 +83,24 @@ TEST(OptionalMonadicTest, Transform) {
8183
beman::optional::optional<const int&> o38r = o38.transform([](int& i) -> const int& { return i; });
8284
EXPECT_TRUE(o38r);
8385
EXPECT_TRUE(*o38r == 42);
86+
87+
// transform and return a non-movable class
88+
using immovable = beman::optional::tests::immovable;
89+
beman::optional::optional<int> o39 = 42;
90+
auto o39r = o39.transform([](int) { return immovable(); });
91+
EXPECT_TRUE(o39r);
92+
93+
beman::optional::optional<int> o40 = 42;
94+
auto o40r = std::move(o40).transform([](int) { return immovable(); });
95+
EXPECT_TRUE(o40r);
96+
97+
const beman::optional::optional<int> o41 = 42;
98+
auto o41r = o41.transform([](int) { return immovable(); });
99+
EXPECT_TRUE(o41r);
100+
101+
const beman::optional::optional<int> o42 = 42;
102+
auto o42r = std::move(o42).transform([](int) { return immovable(); });
103+
EXPECT_TRUE(o42r);
84104
}
85105

86106
TEST(OptionalMonadicTest, TransformConstexpr) {

‎tests/beman/optional/optional_ref_monadic.t.cpp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
#include <beman/optional/optional.hpp>
55

6+
#include <beman/optional/test_types.hpp>
7+
68
#include <gtest/gtest.h>
79

810
namespace {
@@ -89,6 +91,25 @@ TEST(OptionalRefMonadicTest, Transform) {
8991
auto o38r = o38.transform([](int& i) -> const int& { return i; });
9092
EXPECT_TRUE(o38r);
9193
EXPECT_TRUE(*o38r == 42);
94+
95+
// transform and return a non-movable class
96+
using immovable = beman::optional::tests::immovable;
97+
98+
beman::optional::optional<int&> o39 = fortytwo;
99+
auto o39r = o39.transform([](int&) { return immovable(); });
100+
EXPECT_TRUE(o39r);
101+
102+
beman::optional::optional<int&> o40 = fortytwo;
103+
auto o40r = std::move(o40).transform([](int&) { return immovable(); });
104+
EXPECT_TRUE(o40r);
105+
106+
const beman::optional::optional<int&> o41 = fortytwo;
107+
auto o41r = o41.transform([](int&) { return immovable(); });
108+
EXPECT_TRUE(o41r);
109+
110+
const beman::optional::optional<int&> o42 = fortytwo;
111+
auto o42r = std::move(o42).transform([](int&) { return immovable(); });
112+
EXPECT_TRUE(o42r);
92113
}
93114

94115
TEST(OptionalRefMonadicTest, TransformConstexpr) {

0 commit comments

Comments
 (0)