Skip to content

Commit

Permalink
Rename Result::as_ok() to as_value() and add as_value_mut().
Browse files Browse the repository at this point in the history 
Two reasons for renaming:
- as_ok() looks and reads a lot like is_ok() and this is going to be
  particularly painful for non-native speakers I think. But in general
  it looks too similar.
- The value() function is standard in C++ and since this is a method
  that is not mirroring a Rust api (they don't need reference access
  cuz everything moves all the time) there's flexability to match C++
  more there.
  • Loading branch information
@danakj
danakj committed Aug 9, 2023
1 parent 23faa51 commit e6982d6
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 44 deletions.
4 changes: 2 additions & 2 deletions sus/iter/iterator_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,15 +539,15 @@ TEST(Iterator, TryCollect) {
.into_iter()
.try_collect<Vec<i32>>();
static_assert(std::same_as<decltype(collected), Result<Vec<i32>, Error>>);
EXPECT_EQ(collected.as_ok(), sus::Vec<i32>::with(1, 2, 3));
EXPECT_EQ(collected.as_value(), sus::Vec<i32>::with(1, 2, 3));

auto it = sus::Array<Result<i32, Error>, 3>::with(
::sus::ok(1), ::sus::err(ERROR), ::sus::ok(3))
.into_iter();
auto up_to_err = it.try_collect<Vec<i32>>();
EXPECT_EQ(up_to_err, sus::err(ERROR));
auto after_err = it.try_collect<Vec<i32>>();
EXPECT_EQ(after_err.as_ok(), sus::Vec<i32>::with(3));
EXPECT_EQ(after_err.as_value(), sus::Vec<i32>::with(3));
}

auto from =
Expand Down
17 changes: 14 additions & 3 deletions sus/result/result.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -626,13 +626,24 @@ class [[nodiscard]] Result final {
///
/// # Panic
/// Panics if the value is an `Err`.
constexpr const std::remove_reference_t<TUnlessVoid>& as_ok() const&
constexpr const std::remove_reference_t<TUnlessVoid>& as_value() const&
requires(!std::is_void_v<T>)
{
::sus::check(state_ == ResultState::IsOk);
return storage_.ok_;
}
constexpr const std::remove_reference_t<TUnlessVoid>& as_value() && = delete;

/// Returns a mutable reference to the contained `Ok` value.
///
/// # Panic
/// Panics if the value is an `Err`.
constexpr std::remove_reference_t<TUnlessVoid>& as_value_mut() &
requires(!std::is_void_v<T>)
{
::sus::check(state_ == ResultState::IsOk);
return storage_.ok_;
}
/// Returns a const reference to the contained `Err` value.
///
/// # Panic
Expand Down Expand Up @@ -1254,7 +1265,7 @@ template <class T, class E>
struct std::hash<::sus::result::Result<T, E>> {
auto operator()(const ::sus::result::Result<T, E>& u) const noexcept {
if (u.is_ok())
return std::hash<T>()(u.as_ok());
return std::hash<T>()(u.as_value());
else
return std::hash<T>()(u.as_err());
}
Expand Down Expand Up @@ -1291,7 +1302,7 @@ struct fmt::formatter<::sus::result::Result<T, E>, Char> {
if constexpr (std::is_void_v<T>)
out = underlying_ok_.format(ctx);
else
out = underlying_ok_.format(t.as_ok(), ctx);
out = underlying_ok_.format(t.as_value(), ctx);
}
return fmt::format_to(out, ")");
}
Expand Down
95 changes: 56 additions & 39 deletions sus/result/result_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,8 +814,8 @@ TEST(Result, Copy) {
{
auto z = Result<NoCopyMove&, int>::with(m);
auto zz = z;
EXPECT_EQ(&z.as_ok(), &m);
EXPECT_EQ(&zz.as_ok(), &m);
EXPECT_EQ(&z.as_value(), &m);
EXPECT_EQ(&zz.as_value(), &m);
}
{
auto z = Result<NoCopyMove&, int>::with_err(2);
Expand All @@ -827,8 +827,8 @@ TEST(Result, Copy) {
auto z =
Result<NoCopyMove&, NotTriviallyRelocatableCopyableOrMoveable>::with(m);
auto zz = z;
EXPECT_EQ(&z.as_ok(), &m);
EXPECT_EQ(&zz.as_ok(), &m);
EXPECT_EQ(&z.as_value(), &m);
EXPECT_EQ(&zz.as_value(), &m);
}
{
auto z = Result<NoCopyMove&, NotTriviallyRelocatableCopyableOrMoveable>::
Expand All @@ -848,8 +848,8 @@ TEST(Result, Copy) {
auto z = Result<NoCopyMove&, int>::with(m);
auto zz = Result<NoCopyMove&, int>::with_err(2);
zz = z;
EXPECT_EQ(&z.as_ok(), &m);
EXPECT_EQ(&zz.as_ok(), &m);
EXPECT_EQ(&z.as_value(), &m);
EXPECT_EQ(&zz.as_value(), &m);
}
{
auto z =
Expand All @@ -866,8 +866,8 @@ TEST(Result, Copy) {
auto zz = Result<NoCopyMove&, NotTriviallyRelocatableCopyableOrMoveable>::
with_err(NotTriviallyRelocatableCopyableOrMoveable(2));
zz = z;
EXPECT_EQ(&z.as_ok(), &m);
EXPECT_EQ(&zz.as_ok(), &m);
EXPECT_EQ(&z.as_value(), &m);
EXPECT_EQ(&zz.as_value(), &m);
}

// Constexpr copy construct.
Expand Down Expand Up @@ -917,11 +917,11 @@ TEST(Result, Move) {
};
MoveableLvalue lvalue(2);
auto a = Result<MoveableLvalue, i32>::with(lvalue);
EXPECT_EQ(a.as_ok().i, 2);
EXPECT_EQ(a.as_value().i, 2);
EXPECT_EQ(lvalue.i, 2);

auto b = Result<MoveableLvalue, i32>::with(sus::move(lvalue));
EXPECT_EQ(b.as_ok().i, 2);
EXPECT_EQ(b.as_value().i, 2);
EXPECT_EQ(lvalue.i, 0);

{
Expand Down Expand Up @@ -969,18 +969,18 @@ TEST(Result, Move) {
auto m = NoCopyMove();
auto z = Result<NoCopyMove&, int>::with(m);
auto zz = sus::move(z);
EXPECT_EQ(&zz.as_ok(), &m);
EXPECT_EQ(&zz.as_value(), &m);
z = sus::move(zz);
EXPECT_EQ(&z.as_ok(), &m);
EXPECT_EQ(&z.as_value(), &m);
}
{
auto m = NoCopyMove();
auto z =
Result<NoCopyMove&, NotTriviallyRelocatableCopyableOrMoveable>::with(m);
auto zz = sus::move(z);
EXPECT_EQ(&zz.as_ok(), &m);
EXPECT_EQ(&zz.as_value(), &m);
z = sus::move(zz);
EXPECT_EQ(&z.as_ok(), &m);
EXPECT_EQ(&z.as_value(), &m);
}
{
auto m = NoCopyMove();
Expand Down Expand Up @@ -1066,7 +1066,7 @@ TEST(Result, MoveAfterTrivialMove) {
auto rv = Result<NoCopyMove&, i32>::with(m);
auto rv3 = sus::move(rv);
auto rv2 = sus::move(rv3);
EXPECT_EQ(&rv2.as_ok(), &m);
EXPECT_EQ(&rv2.as_value(), &m);
}
}

Expand Down Expand Up @@ -1118,14 +1118,14 @@ TEST(Result, AssignAfterTrivialMove) {
auto rv = Result<NoCopyMove&, i32>::with(m);
auto rv3 = sus::move(rv);
rv = sus::move(rv3);
EXPECT_EQ(&rv.as_ok(), &m);
EXPECT_EQ(&rv.as_value(), &m);
}
{
auto rv = Result<NoCopyMove&, i32>::with(m);
auto rv3 = sus::move(rv);
rv = Result<NoCopyMove&, i32>::with_err(2);
rv = sus::move(rv3);
EXPECT_EQ(&rv.as_ok(), &m);
EXPECT_EQ(&rv.as_value(), &m);
}
}

Expand Down Expand Up @@ -1162,7 +1162,7 @@ TEST(ResultDeathTest, MoveAfterNonTrivialMove) {
auto rv = Result<NoCopyMove&, NonTrivialMove>::with(m);
auto rv3 = sus::move(rv);
rv = sus::move(rv3);
EXPECT_EQ(&rv.as_ok(), &m);
EXPECT_EQ(&rv.as_value(), &m);
}
}

Expand Down Expand Up @@ -1256,14 +1256,14 @@ TEST(Result, AssignAfterNonTrivialMove) {
auto r = Result<NoCopyMove&, NonTrivialMove>::with(m);
auto r3 = sus::move(r);
r = sus::move(r3);
EXPECT_EQ(&r.as_ok(), &m);
EXPECT_EQ(&r.as_value(), &m);
}
{
auto r = Result<NoCopyMove&, NonTrivialMove>::with(m);
auto r3 = sus::move(r);
r = Result<NoCopyMove&, NonTrivialMove>::with_err(NonTrivialMove(1));
r = sus::move(r3);
EXPECT_EQ(&r.as_ok(), &m);
EXPECT_EQ(&r.as_value(), &m);
}

{
Expand All @@ -1279,8 +1279,8 @@ TEST(Result, AssignAfterNonTrivialMove) {
auto r = Result<NoCopyMove&, NonTrivialMove>::with(m);
auto r2 = sus::move(r);
r = Result<NoCopyMove&, NonTrivialMove>::with(m2);
EXPECT_EQ(&r.as_ok(), &m2);
EXPECT_EQ(&r2.as_ok(), &m);
EXPECT_EQ(&r.as_value(), &m2);
EXPECT_EQ(&r2.as_value(), &m);
}
}

Expand Down Expand Up @@ -1310,7 +1310,7 @@ TEST(Result, MoveSelfAssign) {
auto m = NoCopyMove();
auto rm = Result<NoCopyMove&, i32>::with(m);
rm = sus::move(rm);
EXPECT_EQ(&rm.as_ok(), &m);
EXPECT_EQ(&rm.as_value(), &m);
}

TEST(Result, CopySelfAssign) {
Expand Down Expand Up @@ -1339,7 +1339,7 @@ TEST(Result, CopySelfAssign) {
auto m = NoCopyMove();
auto rm = Result<NoCopyMove&, i32>::with(m);
rm = rm;
EXPECT_EQ(&rm.as_ok(), &m);
EXPECT_EQ(&rm.as_value(), &m);
}

TEST(Result, CloneIntoSelfAssign) {
Expand Down Expand Up @@ -1368,7 +1368,7 @@ TEST(Result, CloneIntoSelfAssign) {
auto m = NoCopyMove();
auto rm = Result<NoCopyMove&, i32>::with(m);
sus::clone_into(rm, rm);
EXPECT_EQ(&rm.as_ok(), &m);
EXPECT_EQ(&rm.as_value(), &m);
}

TEST(Result, Iter) {
Expand Down Expand Up @@ -1614,8 +1614,8 @@ TEST(Result, Clone) {
const auto s = Result<Clone, i32>::with(Clone(1));
auto s2 = sus::clone(s);
static_assert(std::same_as<decltype(s2), Result<Clone, i32>>);
EXPECT_EQ(s.as_ok().i, 1_i32);
EXPECT_EQ(s2.as_ok().i, 2_i32);
EXPECT_EQ(s.as_value().i, 1_i32);
EXPECT_EQ(s2.as_value().i, 2_i32);
}
{
const auto s = Result<Clone, i32>::with_err(2);
Expand All @@ -1628,15 +1628,15 @@ TEST(Result, Clone) {
const auto s = Result<Clone, i32>::with(Clone(1));
auto s2 = Result<Clone, i32>::with(Clone(4));
sus::clone_into(s2, s);
EXPECT_EQ(s.as_ok().i, 1_i32);
EXPECT_EQ(s2.as_ok().i, 2_i32);
EXPECT_EQ(s.as_value().i, 1_i32);
EXPECT_EQ(s2.as_value().i, 2_i32);
}
{
const auto s = Result<Clone, i32>::with(Clone(1));
auto s2 = Result<Clone, i32>::with_err(2);
sus::clone_into(s2, s);
EXPECT_EQ(s.as_ok().i, 1_i32);
EXPECT_EQ(s2.as_ok().i, 2_i32);
EXPECT_EQ(s.as_value().i, 1_i32);
EXPECT_EQ(s2.as_value().i, 2_i32);
}
{
const auto s = Result<Clone, i32>::with_err(2);
Expand Down Expand Up @@ -1687,8 +1687,8 @@ TEST(Result, Clone) {
const auto v = Result<NoCopyMove&, i32>::with(m);
auto v2 = sus::clone(v);
static_assert(std::same_as<decltype(v2), Result<NoCopyMove&, i32>>);
EXPECT_EQ(&v.as_ok(), &m);
EXPECT_EQ(&v2.as_ok(), &m);
EXPECT_EQ(&v.as_value(), &m);
EXPECT_EQ(&v2.as_value(), &m);
}
{
const auto v = Result<NoCopyMove&, i32>::with_err(2);
Expand All @@ -1701,15 +1701,15 @@ TEST(Result, Clone) {
const auto v = Result<NoCopyMove&, i32>::with(m);
auto v2 = Result<NoCopyMove&, i32>::with(m);
sus::clone_into(v2, v);
EXPECT_EQ(&v.as_ok(), &m);
EXPECT_EQ(&v2.as_ok(), &m);
EXPECT_EQ(&v.as_value(), &m);
EXPECT_EQ(&v2.as_value(), &m);
}
{
const auto v = Result<NoCopyMove&, i32>::with(m);
auto v2 = Result<NoCopyMove&, i32>::with_err(2);
sus::clone_into(v2, v);
EXPECT_EQ(&v.as_ok(), &m);
EXPECT_EQ(&v2.as_ok(), &m);
EXPECT_EQ(&v.as_value(), &m);
EXPECT_EQ(&v2.as_value(), &m);
}
{
const auto v = Result<NoCopyMove&, i32>::with_err(2);
Expand Down Expand Up @@ -1973,7 +1973,7 @@ TEST(Result, FromProduct) {
sus::Array<Result<i32, E>, 3>::with(sus::ok(2), sus::ok(3), sus::ok(4));
decltype(auto) o = sus::move(a).into_iter().product();
static_assert(std::same_as<decltype(o), sus::Result<i32, E>>);
EXPECT_EQ(o.as_ok(), 2 * 3 * 4);
EXPECT_EQ(o.as_value(), 2 * 3 * 4);
}
}

Expand All @@ -1995,7 +1995,7 @@ TEST(Result, FromSum) {
sus::Array<Result<i32, E>, 3>::with(sus::ok(2), sus::ok(3), sus::ok(4));
decltype(auto) o = sus::move(a).into_iter().sum();
static_assert(std::same_as<decltype(o), sus::Result<i32, E>>);
EXPECT_EQ(o.as_ok(), 2 + 3 + 4);
EXPECT_EQ(o.as_value(), 2 + 3 + 4);
}
}

Expand All @@ -2007,4 +2007,21 @@ TEST(Result, Try) {
false);
}

template <class E>
concept AsValueRvalue = requires {
{ Result<i32, E>::with(2_i32).as_value() };
};

TEST(Result, AsValue) {
auto x = Result<i32, u8>::with(2_i32);
static_assert(std::same_as<decltype(x.as_value()), const i32&>);
EXPECT_EQ(x.as_value(), 2);

static_assert(!AsValueRvalue<u8>);

auto y = Result<i32, u8>::with(2_i32);
static_assert(std::same_as<decltype(y.as_value_mut()), i32&>);
EXPECT_EQ(y.as_value_mut(), 2);
}

} // namespace

0 comments on commit e6982d6

Please sign in to comment.