Skip to content

Commit 0195cf3

Browse files
steve-downeysteve-downey
authored
Merge pull request #128 from dangelog/P2988R12_misc_fixes
Miscellaneous fixes for optional: - optional<T&>::or_else: copy *this instead of creating a new optional - optional::transform: ensure that it works with non-movable payloads - optional<T>::transform() const &&: move the value into the function
2 parents 5d937dd + 6b5137f commit 0195cf3

File tree

3 files changed

+93
-6
lines changed

3 files changed

+93
-6
lines changed

‎include/beman/optional/optional.hpp‎

Lines changed: 27 additions & 6 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), 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
}
@@ -1368,7 +1389,7 @@ constexpr optional<T&> optional<T&>::or_else(F&& f) const {
13681389
using U = std::invoke_result_t<F>;
13691390
static_assert(std::is_same_v<std::remove_cvref_t<U>, optional>, "Result must be an optional");
13701391
if (has_value()) {
1371-
return *value_;
1392+
return *this;
13721393
} else {
13731394
return std::forward<F>(f)();
13741395
}

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

Lines changed: 45 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) {
@@ -323,3 +343,28 @@ TEST(OptionalMonadicTest, Constexpr_or_else) {
323343
constexpr auto test4 = *(std::move(o2).or_else([] { return beman::optional::make_optional(13); })) == 13;
324344
EXPECT_TRUE(test4);
325345
}
346+
347+
constexpr bool optional_or_else_do_not_dereference_impl() {
348+
// create a dangling optional<T&>
349+
int* ptr = new int(42);
350+
beman::optional::optional<int&> iref = *ptr;
351+
delete ptr;
352+
353+
if (!iref)
354+
return false;
355+
356+
const auto or_else_fun = []() { return beman::optional::optional<int&>(); };
357+
358+
// This must not dereference the pointer inside `iref`
359+
beman::optional::optional<int&> iref2 = iref.or_else(or_else_fun);
360+
if (!iref2)
361+
return false;
362+
363+
return true;
364+
}
365+
366+
static_assert(optional_or_else_do_not_dereference_impl());
367+
368+
TEST(OptionalMonadicTest, optional_or_else_do_not_derefence) {
369+
EXPECT_TRUE(optional_or_else_do_not_dereference_impl());
370+
}

‎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)