Skip to content

Commit

Permalink
Remove optional converstions that change the inner type
Browse files Browse the repository at this point in the history 
This is abnormal for C++ containers. In the future std::optional
may gain the ability to hold a reference so we don't want to convert
from references to pointers now since that would mean the wrong thing
happens later or a breaking change would be needed. Instead provide docs
saying how to go from a reference to a pointer or value.
  • Loading branch information
@danakj
danakj committed Dec 25, 2023
1 parent 0bda800 commit 755d82e
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 126 deletions.
81 changes: 0 additions & 81 deletions sus/option/compat_option_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,87 +188,6 @@ TEST(CompatOption, ToOptionalMove) {
}
}

TEST(CompatOption, ToOptionalRef) {
// Explicit.
{
int i = 2;
auto s = sus::Option<const int&>(i);
auto o = std::optional<const int*>(s);
EXPECT_EQ(o.value(), &i);
}
{
int i = 2;
auto s = sus::Option<const int&>(i);
auto o = std::optional<const int*>(s);
EXPECT_EQ(o.value(), &i);
}
{
int i = 2;
auto s = sus::Option<const int&>(i);
auto o = std::optional<const int*>(s);
EXPECT_EQ(o.value(), &i);
}
{
int i = 2;
auto s = sus::Option<int&>(i);
auto o = std::optional<const int*>(s);
EXPECT_EQ(o.value(), &i);
}
{
constexpr auto s = sus::Option<int&>();
constexpr auto o = std::optional<const int*>(s);
EXPECT_EQ(o.has_value(), false);
}
// Implicit.
{
int i = 2;
auto s = sus::Option<const int&>(i);
std::optional<const int*> o = s;
EXPECT_EQ(o.value(), &i);
}
{
int i = 2;
auto s = sus::Option<int&>(i);
std::optional<const int*> o = s;
EXPECT_EQ(o.value(), &i);
}
{
int i = 2;
auto s = sus::Option<int&>(i);
std::optional<int*> o = s;
EXPECT_EQ(o.value(), &i);
}
{
int i = 2;
auto s = sus::Option<int&>(i);
std::optional<const int*> o = s;
EXPECT_EQ(o.value(), &i);
}
{
auto s = sus::Option<int&>();
std::optional<const int*> o = s;
EXPECT_EQ(o.has_value(), false);
}

// TODO: Support .into() back to std? Need a different (ADL? .into<T>()?)
// extension point.
}

TEST(CompatOption, FromOptionalCopyWithConversion) {
static_assert(sus::construct::Into<i32, i64>);

// Move.
constexpr sus::Option<i64> o = sus::into(std::optional<i32>(101));
EXPECT_EQ(o.as_value(), 101_i64);
constexpr sus::Option<i64> o2 = std::optional<i32>(101);
EXPECT_EQ(o2.as_value(), 101_i64);

// Copy.
constexpr auto f = std::optional<i32>(101);
constexpr sus::Option<i64> t = sus::into(f);
EXPECT_EQ(t.as_value(), 101_i64);
}

TEST(CompatOption, Try) {
static_assert(sus::ops::Try<std::optional<i32>>);
static_assert(sus::ops::TryDefault<std::optional<i32>>);
Expand Down
58 changes: 13 additions & 45 deletions sus/option/option.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1891,79 +1891,47 @@ class Option final {
/// Implicit conversion from [`std::optional`](
/// https://en.cppreference.com/w/cpp/utility/optional).
///
/// May convert from `U` in `optional<U>` to `T` in `Option<T>`.
///
/// #[doc.overloads=ctor.optional]
template <class U>
requires(std::convertible_to<U &&, T>)
constexpr Option(std::optional<U>&& s) noexcept
template <std::same_as<T> U>
requires(!std::is_reference_v<T>)
constexpr Option(std::optional<U>&& s) noexcept
: Option() {
if (s.has_value()) t_.set_some(move_to_storage(::sus::move(s).value()));
}
/// #[doc.overloads=ctor.optional]
template <class U>
requires(std::convertible_to<const U&, T>)
constexpr Option(const std::optional<U>& s) noexcept
template <std::same_as<T> U>
requires(!std::is_reference_v<T>)
constexpr Option(const std::optional<U>& s) noexcept
: Option() {
if (s.has_value()) t_.set_some(copy_to_storage(s.value()));
}
/// Implicit conversion to [`std::optional`](
/// https://en.cppreference.com/w/cpp/utility/optional).
///
/// May convert from `T` in `Option<T>` to `U` in `optional<U>`.
///
/// #[doc.overloads=convert.optional]
template <class U>
requires(std::convertible_to<const std::remove_reference_t<T>&, U>)
/// When the option is holding a reference, it will not convert. Use [`map`](
/// $sus::option::Option::map) to convert to a pointer or
/// [`cloned`]($sus::option::Option::cloned) to create a cloned value.
template <std::same_as<T> U>
requires(!std::is_reference_v<T>)
constexpr operator std::optional<U>() const& noexcept {
if (is_some()) {
return std::optional<U>(std::in_place,
return std::optional<T>(std::in_place,
as_value_unchecked(::sus::marker::unsafe_fn));
} else {
return std::nullopt;
}
}
/// #[doc.overloads=convert.optional]
template <class U>
requires(std::convertible_to<T, U>)
template <std::same_as<T> U>
requires(!std::is_reference_v<T>)
constexpr operator std::optional<U>() && noexcept {
if (is_some()) {
return std::optional<U>(
return std::optional<T>(
std::in_place,
::sus::move(*this).unwrap_unchecked(::sus::marker::unsafe_fn));
} else {
return std::nullopt;
}
}
/// #[doc.overloads=convert.optional]
constexpr operator std::optional<const std::remove_reference_t<T>*>()
const& noexcept
requires(std::is_reference_v<T>)
{
if (is_some()) {
return std::optional<const std::remove_reference_t<T>*>(
::sus::mem::addressof(as_value()));
} else {
return std::nullopt;
}
}
// Skip implementing `& noexcept` `&& noexcept` and `const& noexcept` and just
// implement `const& noexcept` which covers them all.
/// #[doc.overloads=convert.optional]
constexpr operator std::optional<std::remove_reference_t<T>*>()
const& noexcept
requires(!std::is_const_v<std::remove_reference_t<T>> &&
std::is_reference_v<T>)
{
if (is_some()) {
return std::optional<std::remove_reference_t<T>*>(
::sus::mem::addressof(as_value_mut()));
} else {
return std::nullopt;
}
}

private:
template <class U>
Expand Down

0 comments on commit 755d82e

Please sign in to comment.