Skip to content

Commit

Permalink
Prevent calling make_optional<int&>(i) or make_optional<const int&>(i)
Browse files Browse the repository at this point in the history
The status quo in C++23 is that
    int i;
    auto o = make_optional<int&>(i);
produces an `optional<int>`, not an `optional<int&>`. This would be horribly
confusing to propagate into C++26 — users would naturally expect to receive
an `optional<int&>`. Worse,
    auto o = make_optional<int&>(std::ref(i));
actually *would* give you an `optional<int&>`, because it would use a different
overload of `make_optional`! So, we propose to add a "Mandates" element to
`make_optional` that requires its explicit template argument (if any) to be
a non-array object type.
This breaks the "status quo" example at the top of this comment (requiring
an Annex C entry); but it successfully prevents the pitfalls.

Thanks to Tomasz Kamiński for the template-programming trick, which matches
the technique used in https://eel.is/c++draft/out.ptr .
  • Loading branch information
Quuxplusone committed Sep 16, 2024
1 parent 33b3dea commit 4e115f1
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 61 deletions.
33 changes: 22 additions & 11 deletions include/Beman/Optional26/optional.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,28 +234,39 @@ template <class T>
inline constexpr bool is_optional<optional<T>> = true;
} // namespace detail

template <typename T>
constexpr optional<std::decay_t<T>>
make_optional(T&& t) noexcept(std::is_nothrow_constructible_v<optional<std::decay_t<T>>, T>)
requires std::is_constructible_v<std::decay_t<T>, T>
template <typename T = void,
typename... Chomp,
typename U,
typename D = std::conditional_t<std::is_void_v<T>, std::decay_t<U>, T>>
constexpr optional<D> make_optional(U&& u) noexcept(std::is_nothrow_constructible_v<D, U>)
requires std::is_constructible_v<D, U>
{
return optional<std::decay_t<T>>{std::forward<T>(t)};
static_assert(sizeof...(Chomp) == 0, "make_optional takes at most one template argument");
if constexpr (!std::is_void_v<T>) {
static_assert(std::is_object_v<T>, "make_optional's template argument must be an object type");
static_assert(!std::is_array_v<T>, "make_optional's template argument must be a non-array object type");
}
return optional<D>(std::forward<U>(u));
}

template <typename T, typename... Args>
constexpr optional<T> make_optional(Args&&... args) noexcept(std::is_nothrow_constructible_v<T, Args...>)
requires std::is_constructible_v<T, Args...>
{
return optional<T>{in_place, std::forward<Args>(args)...};
static_assert(std::is_object_v<T>, "make_optional's template argument must be an object type");
static_assert(!std::is_array_v<T>, "make_optional's template argument must be a non-array object type");
return optional<T>(in_place, std::forward<Args>(args)...);
}

template <typename T, typename _Up, typename... Args>
template <typename T, typename U, typename... Args>
constexpr optional<T>
make_optional(std::initializer_list<_Up> init_list,
Args&&... args) noexcept(std::is_nothrow_constructible_v<T, std::initializer_list<_Up>&, Args...>)
requires std::is_constructible_v<T, std::initializer_list<_Up>&, Args...>
make_optional(std::initializer_list<U> il,
Args&&... args) noexcept(std::is_nothrow_constructible_v<T, std::initializer_list<U>&, Args...>)
requires std::is_constructible_v<T, std::initializer_list<U>&, Args...>
{
return optional<T>{in_place, init_list, std::forward<Args>(args)...};
static_assert(std::is_object_v<T>, "make_optional's template argument must be an object type");
static_assert(!std::is_array_v<T>, "make_optional's template argument must be a non-array object type");
return optional<T>(in_place, il, std::forward<Args>(args)...);
}

template <class T, class U>
Expand Down
2 changes: 1 addition & 1 deletion src/Beman/Optional26/tests/optional.t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ TEST(OptionalTest, MakeOptional) {
EXPECT_TRUE(std::get<1>(o5->t) == 3);

auto i = 42;
auto o6 = beman::optional26::make_optional<int&>(i);
auto o6 = beman::optional26::make_optional(i);
static_assert(std::is_same<decltype(o6), beman::optional26::optional<int>>::value);

EXPECT_TRUE((std::is_same<decltype(o6), beman::optional26::optional<int>>::value));
Expand Down
5 changes: 2 additions & 3 deletions src/Beman/Optional26/tests/optional_constexpr.t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,9 @@ TEST(OptionalConstexprTest, MakeOptional) {
EXPECT_TRUE(std::get<1>(o5->t) == 3);

static constexpr auto i = 42;
constexpr auto o6 = beman::optional26::make_optional<const int&>(i);
static_assert(std::is_same<decltype(o6), const beman::optional26::optional<int>>::value);
constexpr auto o6 = beman::optional26::make_optional(i);
static_assert(std::is_same_v<decltype(o6), const beman::optional26::optional<int>>);

EXPECT_TRUE((std::is_same<decltype(o6), const beman::optional26::optional<int>>::value));
EXPECT_TRUE(o6);
EXPECT_TRUE(*o6 == 42);
}
Expand Down
46 changes: 0 additions & 46 deletions src/Beman/Optional26/tests/optional_ref.t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,52 +349,6 @@ struct takes_init_and_variadic {
takes_init_and_variadic(std::initializer_list<int> l, Args&&... args) : v(l), t(std::forward<Args>(args)...) {}
};

TEST(OptionalRefTest, MakeOptional) {
int var{42};
auto o1 = beman::optional26::make_optional<int&>(var);
auto o2 = beman::optional26::optional<int&>(var);

constexpr bool is_same = std::is_same<decltype(o1), beman::optional26::optional<int>>::value;
EXPECT_TRUE(is_same);
EXPECT_TRUE(o1 == o2);

std::tuple<int, int, int, int> tvar{0, 1, 2, 3};
auto o3 = beman::optional26::make_optional<std::tuple<int, int, int, int>&>(tvar);
EXPECT_TRUE(std::get<0>(*o3) == 0);
EXPECT_TRUE(std::get<1>(*o3) == 1);
EXPECT_TRUE(std::get<2>(*o3) == 2);
EXPECT_TRUE(std::get<3>(*o3) == 3);

std::vector<int> ivec{0, 1, 2, 3};
auto o4 = beman::optional26::make_optional<std::vector<int>&>(ivec);
EXPECT_TRUE(o4.value()[0] == 0);
EXPECT_TRUE(o4.value()[1] == 1);
EXPECT_TRUE(o4.value()[2] == 2);
EXPECT_TRUE(o4.value()[3] == 3);

takes_init_and_variadic tiv{{0, 1}, 2, 3};
auto o5 = beman::optional26::make_optional<takes_init_and_variadic&>(tiv);
EXPECT_TRUE(o5->v[0] == 0);
EXPECT_TRUE(o5->v[1] == 1);
EXPECT_TRUE(std::get<0>(o5->t) == 2);
EXPECT_TRUE(std::get<1>(o5->t) == 3);

auto i = 42;
auto o6 = beman::optional26::make_optional<int&>(i);
static_assert(std::is_same_v<decltype(o6), beman::optional26::optional<int>>);

EXPECT_TRUE((std::is_same_v<decltype(o6), beman::optional26::optional<int>>));
EXPECT_TRUE(o6);
EXPECT_TRUE(*o6 == 42);

// Test the surprising misbehavior of make_optional.
auto co1 = beman::optional26::make_optional<int&>(var);
auto co2 = beman::optional26::make_optional<const int&>(var);

static_assert(std::is_same_v<decltype(co1), beman::optional26::optional<int>>);
static_assert(std::is_same_v<decltype(co2), beman::optional26::optional<const int&>>);
}

TEST(OptionalRefTest, Nullopt) {
beman::optional26::optional<int&> o1 = beman::optional26::nullopt;
beman::optional26::optional<int&> o2{beman::optional26::nullopt};
Expand Down

0 comments on commit 4e115f1

Please sign in to comment.