diff --git a/sus/mem/copy.h b/sus/mem/copy.h index 77735b51a..c5f6c4708 100644 --- a/sus/mem/copy.h +++ b/sus/mem/copy.h @@ -84,4 +84,10 @@ concept TrivialCopy = template concept CopyOrRef = Copy || std::is_reference_v; +/// Matches types which are [`CopyOrRef`]($sus::mem::CopyOrRef) or are `void`. +/// +/// A helper for genertic types which can hold void as a type. +template +concept CopyOrRefOrVoid = CopyOrRef || std::is_void_v; + } // namespace sus::mem diff --git a/sus/mem/move.h b/sus/mem/move.h index 9745ad9a1..be204415b 100644 --- a/sus/mem/move.h +++ b/sus/mem/move.h @@ -63,6 +63,12 @@ concept Move = template concept MoveOrRef = Move || std::is_reference_v; +/// Matches types which are [`MoveOrRef`]($sus::mem::MoveOrRef) or are `void`. +/// +/// A helper for genertic types which can hold void as a type. +template +concept MoveOrRefOrVoid = MoveOrRef || std::is_void_v; + /// Cast `t` to an r-value reference so that it can be used to construct or be /// assigned to a (non-reference) object of type `T`. /// diff --git a/sus/option/__private/storage.h b/sus/option/__private/storage.h index 5c886a254..a0c8ebd85 100644 --- a/sus/option/__private/storage.h +++ b/sus/option/__private/storage.h @@ -161,6 +161,11 @@ struct Storage final { private: union { + // TODO: We can make this T [[no_unique_address]], however then + // we can not construct_at on it. Instead, we would need to only + // construct_at the Storage class, and have the ctor set them both. + // + // See also https://github.com/llvm/llvm-project/issues/70494 T val_; }; State state_ = None; diff --git a/sus/option/option.h b/sus/option/option.h index c6f315bfc..c7fd719dd 100644 --- a/sus/option/option.h +++ b/sus/option/option.h @@ -1991,6 +1991,10 @@ class Option final { using StorageType = std::conditional_t, Storage>, Storage>; + // TODO: We can make this no_unique_address, however... if StorageType has + // tail padding and Option was marked with no_unique_address, then + // constructing T may clobber stuff OUTSIDE the Option. So this can only be + // no_unique_address when StorageType has no tail padding. StorageType t_; sus_class_trivially_relocatable_if_types(::sus::marker::unsafe_fn, diff --git a/sus/result/__private/storage.h b/sus/result/__private/storage.h index 0a4379e04..eb379d7d4 100644 --- a/sus/result/__private/storage.h +++ b/sus/result/__private/storage.h @@ -17,66 +17,524 @@ #pragma once #include +#include +#include "sus/cmp/ord.h" #include "sus/macros/no_unique_address.h" #include "sus/mem/copy.h" #include "sus/mem/move.h" +#include "sus/mem/take.h" namespace sus::result::__private { -enum WithT { kWithT }; -enum WithE { kWithE }; +enum WithT { WITH_T }; +enum WithE { WITH_E }; + +template +struct MaybeNoUniqueAddress { + template + constexpr MaybeNoUniqueAddress(WithT, U&&... v) noexcept + : v(WITH_T, ::sus::forward(v)...) {} + template + constexpr MaybeNoUniqueAddress(WithE, U&&... v) noexcept + : v(WITH_E, ::sus::forward(v)...) {} + + T v; +}; +template +struct MaybeNoUniqueAddress { + template + constexpr MaybeNoUniqueAddress(WithT, U&&... v) noexcept + : v(WITH_T, ::sus::forward(v)...) {} + template + constexpr MaybeNoUniqueAddress(WithE, U&&... v) noexcept + : v(WITH_E, ::sus::forward(v)...) {} + + [[sus_no_unique_address]] T v; +}; + +template +struct StorageVoid { + enum State { Ok, Err, Moved }; + + constexpr StorageVoid(WithT) noexcept : inner_(WITH_T) {} + constexpr StorageVoid(WithE, const E& e) noexcept + requires(std::is_copy_constructible_v) + : inner_(WITH_E, e) {} + constexpr StorageVoid(WithE, E&& e) noexcept + requires(std::is_move_constructible_v) + : inner_(WITH_E, ::sus::move(e)) {} + + constexpr ~StorageVoid() noexcept = default; + + constexpr StorageVoid(const StorageVoid&) noexcept = default; + constexpr StorageVoid& operator=(const StorageVoid&) noexcept = default; + constexpr StorageVoid(StorageVoid&&) noexcept = default; + constexpr StorageVoid& operator=(StorageVoid&&) noexcept = default; + + constexpr bool is_moved() const noexcept { return inner_.v.state == Moved; } + constexpr bool is_ok() const noexcept { return inner_.v.state == Ok; } + constexpr bool is_err() const noexcept { return inner_.v.state == Err; } + + template + constexpr void get_ok() const noexcept {} + constexpr const E& get_err() const noexcept { + return inner_.v.u.err; // + } + + template + constexpr void get_ok_mut() & noexcept {} + constexpr E& get_err_mut() & noexcept { + return inner_.v.u.err; // + } + + template + constexpr void take_ok() & noexcept { + inner_.v.state = Moved; // + } + constexpr E take_err() & noexcept + requires(::sus::mem::Move) + { + inner_.v.state = Moved; + return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, + inner_.v.u.err); + } + + constexpr void drop_ok() & noexcept { + inner_.v.state = Moved; // + } + constexpr void drop_err() & noexcept { + inner_.v.state = Moved; + std::destroy_at(&inner_.v.u.err); + } + + private: + struct Inner { + constexpr explicit Inner(WithT) : u(WITH_T), state(Ok) {} + template + constexpr Inner(WithE, U&& err) + : u(WITH_E, ::sus::forward(err)), state(Err) {} + + constexpr ~Inner() noexcept + requires(std::is_trivially_destructible_v) + = default; + constexpr ~Inner() noexcept + requires(!std::is_trivially_destructible_v) + { + switch (state) { + case Ok: break; + case Err: std::destroy_at(&u.err); break; + case Moved: break; + } + } + + constexpr Inner(const Inner&) + requires(std::is_trivially_copy_constructible_v) + = default; + constexpr Inner(const Inner& o) + requires(!std::is_trivially_copy_constructible_v) + { + switch (o.state) { + case Ok: break; + case Err: std::construct_at(&u.err, o.u.err); break; + case Moved: panic_with_message("Result used after move"); + } + // After construct_at since it may write into the field if it's in tail + // padding. + state = o.state; + } + + constexpr Inner& operator=(const Inner&) + requires(std::is_trivially_copy_assignable_v) + = default; + constexpr Inner& operator=(const Inner& o) + requires(!std::is_trivially_copy_assignable_v) + { + check_with_message(o.state != Moved, "Result used after move"); + if (state == o.state) { + switch (state) { + case Ok: break; + case Err: u.err = o.u.err; break; + case Moved: break; + } + } else { + switch (state) { + case Ok: break; + case Err: std::destroy_at(&u.err); break; + case Moved: break; + } + // If this trips, it means the destructor in this Result moved out + // of the Result that is being assigned from. + check_with_message(o.state != Moved, "Result used after move"); + switch (o.state) { + case Ok: break; + case Err: std::construct_at(&u.err, o.u.err); break; + case Moved: sus::unreachable_unchecked(::sus::marker::unsafe_fn); + } + } + // After construct_at since it may write into the field if it's in tail + // padding. + state = o.state; + return *this; + } + + constexpr Inner(Inner&&) + requires(std::is_trivially_move_constructible_v) + = default; + constexpr Inner(Inner&& o) + requires(!std::is_trivially_move_constructible_v) + { + switch (o.state) { + case Ok: break; + case Err: std::construct_at(&u.err, ::sus::move(o.u.err)); break; + case Moved: panic_with_message("Result used after move"); + } + // After construct_at since it may write into the field if it's in tail + // padding. + state = ::sus::mem::replace(o.state, Moved); + } + + constexpr Inner& operator=(Inner&&) + requires(std::is_trivially_move_assignable_v) + = default; + constexpr Inner& operator=(Inner&& o) + requires(!std::is_trivially_move_assignable_v) + { + check_with_message(o.state != Moved, "Result used after move"); + if (state == o.state) { + switch (state) { + case Ok: break; + case Err: u.err = ::sus::move(o.u.err); break; + case Moved: break; + } + } else { + switch (state) { + case Ok: break; + case Err: std::destroy_at(&u.err); break; + case Moved: break; + } + // If this trips, it means the destructor in this Result moved out + // of the Result that is being assigned from. + check_with_message(o.state != Moved, "Result used after move"); + switch (o.state) { + case Ok: break; + case Err: + std::construct_at(&u.err, ::sus::move(o.u.err)); + std::destroy_at(&o.u.err); + break; + case Moved: sus::unreachable_unchecked(::sus::marker::unsafe_fn); + } + } + // After construct_at since it may write into the field if it's in tail + // padding. + state = ::sus::mem::replace(o.state, Moved); + return *this; + } + + union Union { + constexpr Union() noexcept {} + + constexpr Union(WithT) noexcept {} + template + constexpr Union(WithE, U&& err) noexcept : err(::sus::forward(err)) {} + + constexpr ~Union() noexcept + requires(std::is_trivially_destructible_v) + = default; + constexpr ~Union() noexcept + requires(!std::is_trivially_destructible_v) + {} + + Union(const Union&) + requires(std::is_trivially_copy_constructible_v) + = default; + Union& operator=(const Union&) + requires(std::is_trivially_copy_assignable_v) + = default; + Union(Union&&) + requires(std::is_trivially_move_constructible_v) + = default; + Union& operator=(Union&&) + requires(std::is_trivially_move_assignable_v) + = default; + + [[no_unique_address]] E err; + }; + [[no_unique_address]] Union u; + [[no_unique_address]] State state; + }; + static constexpr bool union_cant_clobber_outside_inner = + ::sus::mem::data_size_of() == ::sus::mem::size_of(); + + [[no_unique_address]] MaybeNoUniqueAddress + inner_; + + sus_class_trivially_relocatable_if_types(::sus::marker::unsafe_fn, + decltype(inner_.v.u.err), + decltype(inner_.v.state)); +}; template -union Storage { - constexpr Storage() {} - - template U> - constexpr Storage(WithT, const U& t) noexcept - requires(sus::mem::Copy) - : ok_(t) {} - template U> - constexpr Storage(WithT, U&& t) noexcept : ok_(::sus::move(t)) {} - constexpr Storage(WithE, const E& e) noexcept - requires(sus::mem::Copy) - : err_(e) {} - constexpr Storage(WithE, E&& e) noexcept : err_(::sus::move(e)) {} - - constexpr ~Storage() - requires(std::is_trivially_destructible_v && - std::is_trivially_destructible_v) - = default; - constexpr ~Storage() - requires(!(std::is_trivially_destructible_v && - std::is_trivially_destructible_v)) +struct StorageNonVoid { + enum State { Ok, Err, Moved }; + + constexpr StorageNonVoid(WithT, const T& t) noexcept + requires(std::is_copy_constructible_v) + : inner_(WITH_T, t) {} + constexpr StorageNonVoid(WithT, T&& t) noexcept + requires(std::is_move_constructible_v) + : inner_(WITH_T, ::sus::move(t)) {} + constexpr StorageNonVoid(WithE, const E& e) noexcept + requires(std::is_copy_constructible_v) + : inner_(WITH_E, e) {} + constexpr StorageNonVoid(WithE, E&& e) noexcept + requires(std::is_move_constructible_v) + : inner_(WITH_E, ::sus::move(e)) {} + + constexpr ~StorageNonVoid() noexcept = default; + + constexpr StorageNonVoid(const StorageNonVoid& o) noexcept = default; + constexpr StorageNonVoid& operator=(const StorageNonVoid&) noexcept = default; + constexpr StorageNonVoid(StorageNonVoid&&) noexcept = default; + constexpr StorageNonVoid& operator=(StorageNonVoid&&) noexcept = default; + + constexpr bool is_moved() const noexcept { return inner_.v.state == Moved; } + constexpr bool is_ok() const noexcept { return inner_.v.state == Ok; } + constexpr bool is_err() const noexcept { return inner_.v.state == Err; } + + template + constexpr const std::remove_reference_t& get_ok() const noexcept { + return inner_.v.u.ok; // + } + constexpr const E& get_err() const noexcept { + return inner_.v.u.err; // + } + + template + constexpr As& get_ok_mut() & noexcept { + return inner_.v.u.ok; // + } + constexpr E& get_err_mut() & noexcept { + return inner_.v.u.err; // + } + + template + constexpr As take_ok() & noexcept + requires(::sus::mem::Move) + { + inner_.v.state = Moved; + return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, + inner_.v.u.ok); + } + constexpr E take_err() & noexcept + requires(::sus::mem::Move) { - // Destruction is handled in Result in this case, but a destructor needs to - // exist here. + inner_.v.state = Moved; + return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, + inner_.v.u.err); } - constexpr Storage(const Storage&) noexcept - requires(std::is_trivially_copy_constructible_v && - std::is_trivially_copy_constructible_v) - = default; - constexpr Storage& operator=(const Storage&) - requires(std::is_trivially_copy_assignable_v && - std::is_trivially_copy_assignable_v) - = default; - - constexpr Storage(Storage&&) - requires(std::is_trivially_move_constructible_v && - std::is_trivially_move_constructible_v) - = default; - constexpr Storage& operator=(Storage&&) - requires(std::is_trivially_move_assignable_v && - std::is_trivially_move_assignable_v) - = default; - - constexpr inline void destroy_ok() noexcept { ok_.~T(); } - constexpr inline void destroy_err() noexcept { err_.~E(); } - - [[sus_no_unique_address]] T ok_; - [[sus_no_unique_address]] E err_; + constexpr void drop_ok() & noexcept { + inner_.v.state = Moved; // + std::destroy_at(&inner_.v.u.ok); + } + constexpr void drop_err() & noexcept { + inner_.v.state = Moved; + std::destroy_at(&inner_.v.u.err); + } + + private: + public: + struct Inner { + template + constexpr Inner(WithT, U&& ok) + : u(WITH_T, ::sus::forward(ok)), state(Ok) {} + template + constexpr Inner(WithE, U&& err) + : u(WITH_E, ::sus::forward(err)), state(Err) {} + + constexpr ~Inner() noexcept + requires(std::is_trivially_destructible_v && + std::is_trivially_destructible_v) + = default; + constexpr ~Inner() noexcept + requires(!std::is_trivially_destructible_v || + !std::is_trivially_destructible_v) + { + switch (state) { + case Ok: std::destroy_at(&u.ok); break; + case Err: std::destroy_at(&u.err); break; + case Moved: break; + } + } + + constexpr Inner(const Inner&) + requires(std::is_trivially_copy_constructible_v && + std::is_trivially_copy_constructible_v) + = default; + constexpr Inner(const Inner& o) + requires(!std::is_trivially_copy_constructible_v || + !std::is_trivially_copy_constructible_v) + { + switch (o.state) { + case Ok: std::construct_at(&u.ok, o.u.ok); break; + case Err: std::construct_at(&u.err, o.u.err); break; + case Moved: panic_with_message("Result used after move"); + } + // After construct_at since it may write into the field if it's in tail + // padding. + state = o.state; + } + + constexpr Inner& operator=(const Inner&) + requires(std::is_trivially_copy_assignable_v && + std::is_trivially_copy_assignable_v) + = default; + constexpr Inner& operator=(const Inner& o) + requires(!std::is_trivially_copy_assignable_v || + !std::is_trivially_copy_assignable_v) + { + check_with_message(o.state != Moved, "Result used after move"); + if (state == o.state) { + switch (state) { + case Ok: u.ok = o.u.ok; break; + case Err: u.err = o.u.err; break; + case Moved: break; + } + } else { + switch (state) { + case Ok: std::destroy_at(&u.ok); break; + case Err: std::destroy_at(&u.err); break; + case Moved: break; + } + // If this trips, it means the destructor in this Result moved out + // of the Result that is being assigned from. + check_with_message(o.state != Moved, "Result used after move"); + switch (o.state) { + case Ok: std::construct_at(&u.ok, o.u.ok); break; + case Err: std::construct_at(&u.err, o.u.err); break; + case Moved: sus::unreachable_unchecked(::sus::marker::unsafe_fn); + } + } + // After construct_at since it may write into the field if it's in tail + // padding. + state = o.state; + return *this; + } + + constexpr Inner(Inner&&) + requires(std::is_trivially_move_constructible_v && + std::is_trivially_move_constructible_v) + = default; + constexpr Inner(Inner&& o) + requires(!std::is_trivially_move_constructible_v || + !std::is_trivially_move_constructible_v) + { + switch (o.state) { + case Ok: std::construct_at(&u.ok, ::sus::move(o.u.ok)); break; + case Err: std::construct_at(&u.err, ::sus::move(o.u.err)); break; + case Moved: panic_with_message("Result used after move"); + } + // After construct_at since it may write into the field if it's in tail + // padding. + state = ::sus::mem::replace(o.state, Moved); + } + + constexpr Inner& operator=(Inner&&) + requires(std::is_trivially_move_assignable_v && + std::is_trivially_move_assignable_v) + = default; + constexpr Inner& operator=(Inner&& o) + requires(!std::is_trivially_move_assignable_v || + !std::is_trivially_move_assignable_v) + { + check_with_message(o.state != Moved, "Result used after move"); + if (state == o.state) { + switch (state) { + case Ok: u.ok = ::sus::move(o.u.ok); break; + case Err: u.err = ::sus::move(o.u.err); break; + case Moved: break; + } + } else { + switch (state) { + case Ok: std::destroy_at(&u.ok); break; + case Err: std::destroy_at(&u.err); break; + case Moved: break; + } + // If this trips, it means the destructor in this Result moved out + // of the Result that is being assigned from. + check_with_message(o.state != Moved, "Result used after move"); + switch (o.state) { + case Ok: + std::construct_at(&u.ok, ::sus::move(o.u.ok)); + std::destroy_at(&o.u.ok); + break; + case Err: + std::construct_at(&u.err, ::sus::move(o.u.err)); + std::destroy_at(&o.u.err); + break; + case Moved: sus::unreachable_unchecked(::sus::marker::unsafe_fn); + } + } + // After construct_at since it may write into the field if it's in tail + // padding. + state = ::sus::mem::replace(o.state, Moved); + return *this; + } + + union Union { + constexpr Union() noexcept {} + + template + constexpr Union(WithT, U&& ok) : ok(::sus::forward(ok)) {} + template + constexpr Union(WithE, U&& err) : err(::sus::forward(err)) {} + + constexpr ~Union() noexcept + requires(std::is_trivially_destructible_v && + std::is_trivially_destructible_v) + = default; + constexpr ~Union() noexcept + requires(!std::is_trivially_destructible_v || + !std::is_trivially_destructible_v) + {} + + Union(const Union&) + requires(std::is_trivially_copy_assignable_v && + std::is_trivially_copy_assignable_v) + = default; + Union& operator=(const Union&) + requires(std::is_trivially_copy_assignable_v && + std::is_trivially_copy_assignable_v) + = default; + Union(Union&&) + requires(std::is_trivially_move_constructible_v && + std::is_trivially_move_constructible_v) + = default; + Union& operator=(Union&&) + requires(std::is_trivially_move_assignable_v && + std::is_trivially_move_assignable_v) + = default; + + [[no_unique_address]] T ok; + [[no_unique_address]] E err; + }; + [[no_unique_address]] Union u; + [[no_unique_address]] State state; + }; + static constexpr bool union_cant_clobber_outside_inner = + (::sus::mem::data_size_of() == ::sus::mem::size_of() && + ::sus::mem::data_size_of() == ::sus::mem::size_of()); + + [[no_unique_address]] MaybeNoUniqueAddress + inner_; + + sus_class_trivially_relocatable_if_types(::sus::marker::unsafe_fn, + decltype(inner_.v.u.ok), + decltype(inner_.v.u.err), + decltype(inner_.v.state)); }; } // namespace sus::result::__private diff --git a/sus/result/result.h b/sus/result/result.h index 59d9bc01a..b28606722 100644 --- a/sus/result/result.h +++ b/sus/result/result.h @@ -36,7 +36,6 @@ #include "sus/mem/copy.h" #include "sus/mem/move.h" #include "sus/mem/relocate.h" -#include "sus/mem/replace.h" #include "sus/mem/take.h" #include "sus/option/option.h" #include "sus/result/__private/marker.h" @@ -65,7 +64,6 @@ using ::sus::mem::__private::IsTrivialMoveAssignOrRef; using ::sus::mem::__private::IsTrivialMoveCtorOrRef; using ::sus::option::__private::StoragePointer; using ::sus::result::__private::ResultState; -using ::sus::result::__private::Storage; /// The representation of an Result's state, which can either be #Ok to /// represent it has a success value, or #Err for when it is holding an error @@ -127,14 +125,14 @@ class [[nodiscard]] Result final { requires(!std::is_void_v && // !std::is_reference_v && // ::sus::mem::Move) - : Result(WITH_OK, move_ok_to_storage(t)) {} + : Result(WITH_OK, ::sus::move(t)) {} /// #[doc.overloads=ctor.ok] template U> explicit constexpr Result(U&& t sus_lifetimebound) noexcept requires(!std::is_void_v && // std::is_reference_v && // sus::construct::SafelyConstructibleFromReference) - : Result(WITH_OK, move_ok_to_storage(t)) {} + : Result(WITH_OK, make_ok_storage(::sus::forward(t))) {} /// Construct an Result that is holding the given error value. static constexpr inline Result with_err(const E& e) noexcept @@ -148,32 +146,13 @@ class [[nodiscard]] Result final { return Result(WITH_ERR, ::sus::move(e)); } - /// Destructor for the Result. + /// Destructor for the `Result`. /// - /// Destroys the Ok or Err value contained within the Result. + /// Destroys the Ok or Err value contained within the `Result`. /// - /// If T and E can be trivially destroyed, we don't need to explicitly destroy - /// them, so we can use the default destructor, which allows Result to - /// also be trivially destroyed. - constexpr ~Result() - requires((std::is_void_v || IsTrivialDtorOrRef) && - IsTrivialDtorOrRef) - = default; - - constexpr inline ~Result() noexcept - requires( - // clang-format off - !((std::is_void_v || IsTrivialDtorOrRef) && - IsTrivialDtorOrRef) - // clang-format on - ) - { - switch (state_) { - case ResultState::IsMoved: break; - case ResultState::IsOk: storage_.destroy_ok(); break; - case ResultState::IsErr: storage_.destroy_err(); break; - } - } + /// If T and E can be trivially destroyed, `Result` can also be + /// trivially destroyed. + constexpr ~Result() = default; /// Copy constructor for `Result` which satisfies /// [`sus::mem::Copy>`](sus-mem-Copy.html) if @@ -185,43 +164,12 @@ class [[nodiscard]] Result final { /// /// #[doc.overloads=copy] constexpr Result(const Result&) - requires((std::is_void_v || ::sus::mem::CopyOrRef) && - ::sus::mem::Copy && - (std::is_void_v || IsTrivialCopyCtorOrRef) && - IsTrivialCopyCtorOrRef) + requires(::sus::mem::CopyOrRefOrVoid && ::sus::mem::Copy) = default; - /// #[doc.overloads=copy] - constexpr Result(const Result& rhs) noexcept - requires( - // clang-format off - (std::is_void_v || ::sus::mem::CopyOrRef) && - ::sus::mem::Copy && - !((std::is_void_v || IsTrivialCopyCtorOrRef) && - IsTrivialCopyCtorOrRef) - // clang-format on - ) - : state_(rhs.state_) { - ::sus::check(state_ != ResultState::IsMoved); - switch (state_) { - case ResultState::IsOk: - if constexpr (!std::is_void_v) - std::construct_at(&storage_.ok_, rhs.storage_.ok_); - break; - case ResultState::IsErr: - std::construct_at(&storage_.err_, rhs.storage_.err_); - break; - case ResultState::IsMoved: - // SAFETY: The state_ is verified to be Ok or Err at the top of the - // function. - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); - } - } - /// #[doc.overloads=copy] constexpr Result(const Result&) - requires(!((std::is_void_v || ::sus::mem::CopyOrRef) && - ::sus::mem::Copy)) + requires(!::sus::mem::CopyOrRefOrVoid || !::sus::mem::Copy) = delete; /// Copy assignment for `Result` which satisfies @@ -234,130 +182,36 @@ class [[nodiscard]] Result final { /// /// #[doc.overloads=copy] constexpr Result& operator=(const Result& o) - requires((std::is_void_v || ::sus::mem::CopyOrRef) && - ::sus::mem::Copy && - (std::is_void_v || IsTrivialCopyAssignOrRef) && - IsTrivialCopyAssignOrRef) + requires(::sus::mem::CopyOrRefOrVoid && ::sus::mem::Copy) = default; - /// #[doc.overloads=copy] - constexpr Result& operator=(const Result& o) noexcept - requires( - // clang-format off - (std::is_void_v || ::sus::mem::CopyOrRef) && ::sus::mem::Copy && - !((std::is_void_v || IsTrivialCopyAssignOrRef) && - IsTrivialCopyAssignOrRef) - // clang-format on - ) - { - check(o.state_ != ResultState::IsMoved); - switch (state_) { - case ResultState::IsOk: - switch (state_ = o.state_) { - case ResultState::IsOk: - if constexpr (!std::is_void_v) { - storage_.ok_ = o.storage_.ok_; - } - break; - case ResultState::IsErr: - storage_.destroy_ok(); - std::construct_at(&storage_.err_, o.storage_.err_); - break; - // SAFETY: This condition is check()'d at the top of the function. - case ResultState::IsMoved: - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); - } - break; - case ResultState::IsErr: - switch (state_ = o.state_) { - case ResultState::IsErr: storage_.err_ = o.storage_.err_; break; - case ResultState::IsOk: - storage_.destroy_err(); - if constexpr (!std::is_void_v) - std::construct_at(&storage_.ok_, o.storage_.ok_); - break; - // SAFETY: This condition is check()'d at the top of the function. - case ResultState::IsMoved: - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); - } - break; - case ResultState::IsMoved: - switch (state_ = o.state_) { - case ResultState::IsErr: - std::construct_at(&storage_.err_, o.storage_.err_); - break; - case ResultState::IsOk: - if constexpr (!std::is_void_v) - std::construct_at(&storage_.ok_, o.storage_.ok_); - break; - // SAFETY: This condition is check()'d at the top of the function. - case ResultState::IsMoved: - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); - } - break; - } - return *this; - } - /// #[doc.overloads=copy] constexpr Result& operator=(const Result&) - requires(!((std::is_void_v || ::sus::mem::CopyOrRef) && - ::sus::mem::Copy)) + requires(!::sus::mem::CopyOrRefOrVoid || !::sus::mem::Copy) = delete; /// Move constructor for `Result` which satisfies - /// [`sus::mem::Move>`](sus-mem-Move.html) if - /// [`Move`](sus-mem-Move.html) and - /// [`Move`](sus-mem-Move.html) are satisfied. + /// [`Move`]($sus::mem::Move), if `T` and `E` both satisfy + /// [`Move`]($sus::mem::Move). /// /// If `T` and `E` can be trivially move-constructed, then `Result` can /// also be trivially move-constructed. When trivially-moved, the `Result` is - /// copied on move, and the moved-from Result is unchanged but should still + /// copied on move, and the moved-from `Result` is unchanged but should still /// not be used thereafter without reinitializing it. /// /// #[doc.overloads=move] constexpr Result(Result&&) - requires((std::is_void_v || ::sus::mem::MoveOrRef) && - ::sus::mem::Move && - (std::is_void_v || IsTrivialMoveCtorOrRef) && - IsTrivialMoveCtorOrRef) + requires(::sus::mem::MoveOrRefOrVoid && ::sus::mem::Move) = default; - /// #[doc.overloads=move] - constexpr Result(Result&& rhs) noexcept - requires( - // clang-format off - (std::is_void_v || ::sus::mem::MoveOrRef) && ::sus::mem::Move && - !((std::is_void_v || IsTrivialMoveCtorOrRef) && - IsTrivialMoveCtorOrRef) - // clang-format on - ) - : state_(::sus::mem::replace(rhs.state_, ResultState::IsMoved)) { - ::sus::check(state_ != ResultState::IsMoved); - switch (state_) { - case ResultState::IsOk: - std::construct_at(&storage_.ok_, ::sus::move(rhs.storage_.ok_)); - break; - case ResultState::IsErr: - std::construct_at(&storage_.err_, ::sus::move(rhs.storage_.err_)); - break; - case ResultState::IsMoved: - // SAFETY: The state_ is verified to be Ok or Err at the top of the - // function. - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); - } - } - /// #[doc.overloads=move] constexpr Result(Result&&) - requires(!((std::is_void_v || ::sus::mem::MoveOrRef) && - ::sus::mem::Move)) + requires(!::sus::mem::MoveOrRefOrVoid || !::sus::mem::Move) = delete; /// Move assignment for `Result` which satisfies - /// [`sus::mem::Move>`](sus-mem-Move.html) if - /// [`Move`](sus-mem-Move.html) and - /// [`Move`](sus-mem-Move.html) are satisfied. + /// [`Move`]($sus::mem::Move), if `T` and `E` both satisfy + /// [`Move`]($sus::mem::Move). /// /// If `T` and `E` can be trivially move-assigned, then `Result` can /// also be trivially move-assigned. When trivially-moved, the `Result` is @@ -365,121 +219,55 @@ class [[nodiscard]] Result final { /// not be used thereafter without reinitializing it. /// /// #[doc.overloads=move] - constexpr Result& operator=(Result&& o) - requires((std::is_void_v || ::sus::mem::MoveOrRef) && - ::sus::mem::Move && - (std::is_void_v || IsTrivialMoveAssignOrRef) && - IsTrivialMoveAssignOrRef) + constexpr Result& operator=(Result&&) + requires(::sus::mem::MoveOrRefOrVoid && ::sus::mem::Move) = default; /// #[doc.overloads=move] - constexpr Result& operator=(Result&& o) noexcept - requires( - // clang-format off - (std::is_void_v || ::sus::mem::MoveOrRef) && ::sus::mem::Move && - !((std::is_void_v || IsTrivialMoveAssignOrRef) && - IsTrivialMoveAssignOrRef) - // clang-format on - ) - { - check(o.state_ != ResultState::IsMoved); - switch (state_) { - case ResultState::IsOk: - switch (state_ = ::sus::mem::replace(o.state_, ResultState::IsMoved)) { - case ResultState::IsOk: - storage_.ok_ = ::sus::move(o.storage_.ok_); - break; - case ResultState::IsErr: - storage_.destroy_ok(); - std::construct_at(&storage_.err_, ::sus::move(o.storage_.err_)); - break; - // SAFETY: This condition is check()'d at the top of the function. - case ResultState::IsMoved: - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); - } - break; - case ResultState::IsErr: - switch (state_ = ::sus::mem::replace(o.state_, ResultState::IsMoved)) { - case ResultState::IsErr: - storage_.err_ = ::sus::move(o.storage_.err_); - break; - case ResultState::IsOk: - storage_.destroy_err(); - std::construct_at(&storage_.ok_, ::sus::move(o.storage_.ok_)); - break; - // SAFETY: This condition is check()'d at the top of the function. - case ResultState::IsMoved: - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); - } - break; - case ResultState::IsMoved: - switch (state_ = ::sus::mem::replace(o.state_, ResultState::IsMoved)) { - case ResultState::IsErr: - std::construct_at(&storage_.err_, ::sus::move(o.storage_.err_)); - break; - case ResultState::IsOk: - std::construct_at(&storage_.ok_, ::sus::move(o.storage_.ok_)); - break; - // SAFETY: This condition is check()'d at the top of the function. - case ResultState::IsMoved: - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); - } - break; - } - return *this; - } - - /// #[doc.overloads=move] - constexpr Result& operator=(Result&& o) - requires(!(std::is_void_v || ::sus::mem::MoveOrRef) || - !::sus::mem::Move) + constexpr Result& operator=(Result&&) + requires(!::sus::mem::MoveOrRefOrVoid || !::sus::mem::Move) = delete; constexpr Result clone() const& noexcept requires((std::is_void_v || ::sus::mem::Clone) && ::sus::mem::Clone && - !((std::is_void_v || ::sus::mem::CopyOrRef) && - ::sus::mem::Copy)) + !(::sus::mem::CopyOrRefOrVoid && ::sus::mem::Copy)) { - ::sus::check(state_ != ResultState::IsMoved); - switch (state_) { - case ResultState::IsOk: - if constexpr (std::is_void_v) - return Result(WITH_OK); - else - return Result(WITH_OK, ::sus::clone(storage_.ok_)); - case ResultState::IsErr: - return Result(WITH_ERR, ::sus::clone(storage_.err_)); - case ResultState::IsMoved: break; + if (storage_.is_ok()) { + if constexpr (std::is_void_v) + return Result(WITH_OK); + else + return Result(WITH_OK, ::sus::clone(storage_.template get_ok())); + } else if (storage_.is_err()) { + return Result(WITH_ERR, ::sus::clone(storage_.get_err())); + } else { + panic_with_message("Result used after move"); } - // SAFETY: The state_ is verified to be Ok or Err at the top of the - // function. - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); } constexpr void clone_from(const Result& source) & requires((std::is_void_v || ::sus::mem::Clone) && ::sus::mem::Clone && - !((std::is_void_v || ::sus::mem::CopyOrRef) && - ::sus::mem::Copy)) + !(::sus::mem::CopyOrRefOrVoid && ::sus::mem::Copy)) { - ::sus::check(source.state_ != ResultState::IsMoved); - if (&source == this) [[unlikely]] - return; - if (state_ == source.state_) { - switch (state_) { - case ResultState::IsOk: - if constexpr (!std::is_void_v) - ::sus::clone_into(storage_.ok_, source.storage_.ok_); - break; - case ResultState::IsErr: - ::sus::clone_into(storage_.err_, source.storage_.err_); - break; - case ResultState::IsMoved: - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); - } - } else { + if (source.storage_.is_moved()) [[unlikely]] { + panic_with_message("Result used after move"); + } else if (&source == this) [[unlikely]] { + // Nothing to do. + } else if (storage_.is_moved()) { + *this = source.clone(); // Replace the moved self. + } else if (storage_.is_ok() != source.storage_.is_ok()) { + // Moving to a different state, replace everything. *this = source.clone(); + } else { + // Both are in the same state. So recursively clone_into. + if (storage_.is_ok()) { + if constexpr (!std::is_void_v) + ::sus::clone_into(storage_.template get_ok_mut(), + source.storage_.template get_ok()); + } else { + ::sus::clone_into(storage_.get_err_mut(), source.storage_.get_err()); + } } } @@ -589,14 +377,14 @@ class [[nodiscard]] Result final { /// Returns true if the result is `Ok`. constexpr inline bool is_ok() const& noexcept { - ::sus::check(state_ != ResultState::IsMoved); - return state_ == ResultState::IsOk; + ::sus::check(!storage_.is_moved()); + return storage_.is_ok(); } /// Returns true if the result is `Err`. constexpr inline bool is_err() const& noexcept { - ::sus::check(state_ != ResultState::IsMoved); - return state_ == ResultState::IsErr; + ::sus::check(!storage_.is_moved()); + return storage_.is_err(); } /// An operator which returns the state of the Result, either #Ok or #Err. @@ -617,8 +405,11 @@ class [[nodiscard]] Result final { /// } /// ``` constexpr inline operator State() const& noexcept { - ::sus::check(state_ != ResultState::IsMoved); - return static_cast(state_); + ::sus::check(!storage_.is_moved()); + if (storage_.is_ok()) + return State::Ok; + else + return State::Err; } /// Calls `op` if the result is `Ok`, otherwise returns the `Err` value of @@ -627,64 +418,53 @@ class [[nodiscard]] Result final { /// This function can be used for control flow based on `Result` values. template <::sus::fn::FnOnce<::sus::fn::NonVoid(TUnlessVoid&&)> AndFn> requires( - !std::is_void_v && // + // TODO: FnOnce should let us plug in our own type check instead of + // NonVoid. Then this becomes part of the FnOnce constraint. __private::IsResultWithErrType, E>) constexpr sus::fn::ReturnOnce and_then( AndFn op) && noexcept { - ::sus::check(state_ != ResultState::IsMoved); - switch (::sus::mem::replace(state_, ResultState::IsMoved)) { - case ResultState::IsOk: - return ::sus::fn::call_once( - ::sus::move(op), ::sus::mem::take_and_destruct( - ::sus::marker::unsafe_fn, storage_.ok_)); - case ResultState::IsErr: - return sus::fn::ReturnOnce::with_err( - ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.err_)); - case ResultState::IsMoved: break; + if (storage_.is_ok()) { + return ::sus::fn::call_once(::sus::move(op), + storage_.template take_ok()); + } else if (storage_.is_err()) { + return sus::fn::ReturnOnce::with_err(storage_.take_err()); + } else { + panic_with_message("Result used after move"); } - // SAFETY: The state_ is verified to be Ok or Err at the top of the - // function. - sus::unreachable_unchecked(::sus::marker::unsafe_fn); } template <::sus::fn::FnOnce<::sus::fn::NonVoid()> AndFn> requires(std::is_void_v && // + // TODO: FnOnce should let us plug in our own type check instead of + // NonVoid. Then this becomes part of the FnOnce constraint. __private::IsResultWithErrType, E>) constexpr sus::fn::ReturnOnce and_then(AndFn op) && noexcept { - ::sus::check(state_ != ResultState::IsMoved); - switch (::sus::mem::replace(state_, ResultState::IsMoved)) { - case ResultState::IsOk: return ::sus::fn::call_once(::sus::move(op)); - case ResultState::IsErr: - return sus::fn::ReturnOnce::with_err( - ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.err_)); - case ResultState::IsMoved: break; + if (storage_.is_ok()) { + storage_.drop_ok(); + return ::sus::fn::call_once(::sus::move(op)); + } else if (storage_.is_err()) { + return sus::fn::ReturnOnce::with_err(storage_.take_err()); + } else { + panic_with_message("Result used after move"); } - // SAFETY: The state_ is verified to be Ok or Err at the top of the - // function. - sus::unreachable_unchecked(::sus::marker::unsafe_fn); } - /// Converts from `Result` to `Option`. + /// Converts from `Result` to [`Option`]($sus::option::Option). /// /// Converts self into an `Option`, consuming self, and discarding the /// error, if any. + /// + /// [`Option`]($sus::option::Option) can not hold `void`, so this method is + /// not present on `Result`. constexpr inline Option ok() && noexcept requires(!std::is_void_v) { - ::sus::check(state_ != ResultState::IsMoved); - switch (::sus::mem::replace(state_, ResultState::IsMoved)) { - case ResultState::IsOk: - // SAFETY: The static_cast is needed to convert the pointer storage type - // to a `const T&`, which does not create a temporary as it's converting - // a pointer to a reference. - return Option(static_cast(::sus::mem::take_and_destruct( - ::sus::marker::unsafe_fn, storage_.ok_))); - case ResultState::IsErr: storage_.destroy_err(); return Option(); - case ResultState::IsMoved: break; + if (storage_.is_ok()) { + return Option(storage_.template take_ok()); + } else if (storage_.is_err()) { + storage_.drop_err(); + return Option(); + } else { + panic_with_message("Result used after move"); } - // SAFETY: The state_ is verified to be Ok or Err at the top of the - // function. - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); } /// Converts from `Result` to `Option`. @@ -692,17 +472,14 @@ class [[nodiscard]] Result final { /// Converts self into an `Option`, consuming self, and discarding the /// success value, if any. constexpr inline Option err() && noexcept { - ::sus::check(state_ != ResultState::IsMoved); - switch (::sus::mem::replace(state_, ResultState::IsMoved)) { - case ResultState::IsOk: storage_.destroy_ok(); return Option(); - case ResultState::IsErr: - return Option(::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.err_)); - case ResultState::IsMoved: break; + if (storage_.is_ok()) { + storage_.drop_ok(); + return Option(); + } else if (storage_.is_err()) { + return Option(storage_.take_err()); + } else { + panic_with_message("Result used after move"); } - // SAFETY: The state_ is verified to be Ok or Err at the top of the - // function. - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); } /// Returns a const reference to the contained `Ok` value. @@ -712,18 +489,17 @@ class [[nodiscard]] Result final { constexpr const std::remove_reference_t& as_value() const& requires(!std::is_void_v) { - if (state_ != ResultState::IsOk) [[unlikely]] { - if (state_ == ResultState::IsErr) { - if constexpr (fmt::is_formattable::value) { - panic_with_message(fmt::to_string(storage_.err_)); - } else { - panic_with_message("Result has error state"); - } + if (storage_.is_ok()) [[likely]] { + return storage_.template get_ok(); + } else if (storage_.is_err()) { + if constexpr (fmt::is_formattable::value) { + panic_with_message(fmt::to_string(storage_.get_err())); } else { - panic_with_message("Result used after move"); + panic_with_message("Result has error state"); } + } else { + panic_with_message("Result used after move"); } - return storage_.ok_; } constexpr const std::remove_reference_t& as_value() && = delete; @@ -734,42 +510,38 @@ class [[nodiscard]] Result final { constexpr std::remove_reference_t& as_value_mut() & requires(!std::is_void_v) { - if (state_ != ResultState::IsOk) [[unlikely]] { - if (state_ == ResultState::IsErr) { - if constexpr (fmt::is_formattable::value) { - panic_with_message(fmt::to_string(storage_.err_)); - } else { - panic_with_message("Result has error state"); - } + if (storage_.is_ok()) [[likely]] { + return storage_.template get_ok_mut(); + } else if (storage_.is_err()) { + if constexpr (fmt::is_formattable::value) { + panic_with_message(fmt::to_string(storage_.get_err())); } else { - panic_with_message("Result used after move"); + panic_with_message("Result has error state"); } + } else { + panic_with_message("Result used after move"); } - return storage_.ok_; } /// Returns a const reference to the contained `Err` value. /// /// # Panics /// Panics if the value is an `Ok` or the Result is moved from. - constexpr const std::remove_reference_t& as_err() const& { - if (state_ != ResultState::IsErr) [[unlikely]] { - if (state_ == ResultState::IsOk) { - if constexpr (!std::is_void_v) { - if constexpr (fmt::is_formattable::value) { - panic_with_message(fmt::to_string(storage_.ok_)); - } else { - panic_with_message("Result has ok state"); - } - } else { - panic_with_message("Result has ok state"); - } + constexpr const E& as_err() const& { + if (storage_.is_err()) [[likely]] { + return storage_.get_err(); + } else if (storage_.is_ok()) { + if constexpr (std::is_void_v) { + panic_with_message("Result has ok state"); + } else if constexpr (!fmt::is_formattable::value) { + panic_with_message("Result has ok state"); } else { - panic_with_message("Result used after move"); + panic_with_message(fmt::to_string(storage_.template get_ok())); } + } else { + panic_with_message("Result used after move"); } - return storage_.err_; } - constexpr const std::remove_reference_t& as_err() && = delete; + constexpr const E& as_err() && = delete; /// Returns the contained `Ok` value, consuming the self value. /// @@ -781,21 +553,16 @@ class [[nodiscard]] Result final { /// # Panics /// Panics if the value is an `Err` or the Result is moved from. constexpr inline T unwrap() && noexcept { - ResultState was = ::sus::mem::replace(state_, ResultState::IsMoved); - if (was != ResultState::IsOk) [[unlikely]] { - if (was == ResultState::IsErr) { - if constexpr (fmt::is_formattable::value) { - panic_with_message(fmt::to_string(storage_.err_)); - } else { - panic_with_message("Result has error state"); - } + if (storage_.is_ok()) [[likely]] { + return storage_.template take_ok(); + } else if (storage_.is_err()) { + if constexpr (fmt::is_formattable::value) { + panic_with_message(fmt::to_string(storage_.get_err())); } else { - panic_with_message("Result used after move"); + panic_with_message("Result has error state"); } - } - if constexpr (!std::is_void_v) { - return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.ok_); + } else { + panic_with_message("Result used after move"); } } @@ -809,21 +576,16 @@ class [[nodiscard]] Result final { /// Panics if the value is an Err, with a panic message including the passed /// message, and the content of the Err. constexpr inline T expect(const char* msg) && noexcept { - ResultState was = ::sus::mem::replace(state_, ResultState::IsMoved); - if (was != ResultState::IsOk) [[unlikely]] { - if (was == ResultState::IsErr) { - if constexpr (fmt::is_formattable::value) { - panic_with_message(fmt::format("{}: {}", msg, storage_.err_)); - } else { - panic_with_message(msg); - } + if (storage_.is_ok()) [[likely]] { + return storage_.template take_ok(); + } else if (storage_.is_err()) { + if constexpr (fmt::is_formattable::value) { + panic_with_message(fmt::format("{}: {}", msg, storage_.get_err())); } else { - panic_with_message("Result used after move"); + panic_with_message(msg); } - } - if constexpr (!std::is_void_v) { - return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.ok_); + } else { + panic_with_message("Result used after move"); } } @@ -835,17 +597,16 @@ class [[nodiscard]] Result final { requires(!std::is_reference_v && (std::is_void_v || ::sus::construct::Default)) { - ResultState was = ::sus::mem::replace(state_, ResultState::IsMoved); - check_with_message( - was != ResultState::IsMoved, - "called `Result::unwrap_or_default()` on a moved Result"); - if constexpr (!std::is_void_v) { - if (was == ResultState::IsOk) { - return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.ok_); - } else { + if (storage_.is_ok()) { + return storage_.template take_ok(); + } else if (storage_.is_err()) { + storage_.drop_err(); + if constexpr (!std::is_void_v) return T(); - } + else + return; + } else { + panic_with_message("Result used after move"); } } @@ -857,7 +618,7 @@ class [[nodiscard]] Result final { /// Behavior. constexpr inline T unwrap_unchecked( ::sus::marker::UnsafeFnMarker) && noexcept { - if (state_ != ResultState::IsOk) { + if (!storage_.is_ok()) { // This enables Clang to drop any branches that were used to construct // the Result. Without this, the optimizer keeps the whole Result // construction, possibly because the `state_` gets clobbered below? @@ -865,11 +626,7 @@ class [[nodiscard]] Result final { // greatly when the compiler is informed about the UB here. sus::unreachable_unchecked(::sus::marker::unsafe_fn); } - state_ = ResultState::IsMoved; - if constexpr (!std::is_void_v) { - return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.ok_); - } + return storage_.template take_ok(); } /// Returns the contained `Err` value, consuming the self value. @@ -877,24 +634,19 @@ class [[nodiscard]] Result final { /// # Panics /// Panics if the value is an `Ok` or the Result is moved from. constexpr inline E unwrap_err() && noexcept { - ResultState was = ::sus::mem::replace(state_, ResultState::IsMoved); - if (was != ResultState::IsErr) [[unlikely]] { - if (was == ResultState::IsOk) { - if constexpr (!std::is_void_v) { - if constexpr (fmt::is_formattable::value) { - panic_with_message(fmt::to_string(storage_.ok_)); - } else { - panic_with_message("Result has ok state"); - } - } else { - panic_with_message("Result has ok state"); - } + if (storage_.is_err()) [[likely]] { + return storage_.take_err(); + } else if (storage_.is_ok()) { + if constexpr (std::is_void_v) { + panic_with_message("Result has ok state"); + } else if constexpr (!fmt::is_formattable::value) { + panic_with_message("Result has ok state"); } else { - panic_with_message("Result used after move"); + panic_with_message(fmt::to_string(storage_.template get_ok())); } + } else { + panic_with_message("Result used after move"); } - return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.err_); } /// Returns the contained `Err` value, consuming the self value, without @@ -905,14 +657,12 @@ class [[nodiscard]] Result final { /// Behavior. constexpr inline E unwrap_err_unchecked( ::sus::marker::UnsafeFnMarker) && noexcept { - if (state_ != ResultState::IsErr) { + if (!storage_.is_err()) { // Match the code in unwrap_unchecked, and tell the compiler that the // `state_` is an Err before clobbering it. sus::unreachable_unchecked(::sus::marker::unsafe_fn); } - state_ = ResultState::IsMoved; - return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.err_); + return storage_.take_err(); } /// Returns the contained `Ok` value or computes it from a closure. @@ -927,14 +677,13 @@ class [[nodiscard]] Result final { /// auto err = sus::Result::with_err(ItsHappening); /// sus::check(sus::move(err).unwrap_or_else(conv) == -1); /// ``` - template <::sus::fn::FnOnce F> - constexpr T unwrap_or_else(F&& op) && noexcept { - if (is_ok()) { - return ::sus::move(*this).unwrap_unchecked(::sus::marker::unsafe_fn); + constexpr T unwrap_or_else(::sus::fn::FnOnce auto op) && noexcept { + if (storage_.is_ok()) { + return storage_.template take_ok(); + } else if (storage_.is_err()) { + return ::sus::fn::call_once(::sus::move(op), storage_.take_err()); } else { - return ::sus::fn::call_once( - ::sus::move(op), - ::sus::move(*this).unwrap_err_unchecked(::sus::marker::unsafe_fn)); + panic_with_message("Result used after move"); } } @@ -943,17 +692,15 @@ class [[nodiscard]] Result final { iter() const& noexcept requires(!std::is_void_v) { - ::sus::check(state_ != ResultState::IsMoved); - if (state_ == ResultState::IsOk) { + if (storage_.is_ok()) { return ::sus::option::OptionIter&>( - // SAFETY: The static_cast is needed to convert the pointer storage - // type to a `const T&`, which does not create a temporary as it's - // converting a pointer to a reference. Option&>( - static_cast&>(storage_.ok_))); - } else { + storage_.template get_ok())); + } else if (storage_.is_err()) { return ::sus::option::OptionIter&>( Option&>()); + } else { + panic_with_message("Result used after move"); } } constexpr ::sus::option::OptionIter< @@ -961,58 +708,57 @@ class [[nodiscard]] Result final { iter() && noexcept requires(!std::is_void_v && std::is_reference_v) { - ::sus::check(state_ != ResultState::IsMoved); - if (::sus::mem::replace(state_, ResultState::IsMoved) == - ResultState::IsOk) { + if (storage_.is_ok()) { return ::sus::option::OptionIter&>( - // SAFETY: The static_cast is needed to convert the pointer storage - // type to a `const T&`, which does not create a temporary as it's - // converting a pointer to a reference. Option&>( - static_cast&>(storage_.ok_))); - } else { - storage_.destroy_err(); + storage_.template take_ok())); + } else if (storage_.is_err()) { + storage_.drop_err(); return ::sus::option::OptionIter&>( Option&>()); + } else { + panic_with_message("Result used after move"); } } constexpr ::sus::option::OptionIter iter_mut() & noexcept requires(!std::is_void_v) { - ::sus::check(state_ != ResultState::IsMoved); - if (state_ == ResultState::IsOk) { - return ::sus::option::OptionIter(Option(storage_.ok_)); - } else { + if (storage_.is_ok()) { + return ::sus::option::OptionIter( + Option(storage_.template get_ok_mut())); + } else if (storage_.is_err()) { return ::sus::option::OptionIter(Option()); + } else { + panic_with_message("Result used after move"); } } constexpr ::sus::option::OptionIter iter_mut() && noexcept requires(!std::is_void_v && // std::is_reference_v) { - ::sus::check(state_ != ResultState::IsMoved); - if (::sus::mem::replace(state_, ResultState::IsMoved) == - ResultState::IsOk) { - return ::sus::option::OptionIter(Option(storage_.ok_)); - } else { - storage_.destroy_err(); + if (storage_.is_ok()) { + return ::sus::option::OptionIter( + Option(storage_.template take_ok())); + } else if (storage_.is_err()) { + storage_.drop_err(); return ::sus::option::OptionIter(Option()); + } else { + panic_with_message("Result used after move"); } } constexpr ::sus::option::OptionIter into_iter() && noexcept requires(!std::is_void_v) { - ::sus::check(state_ != ResultState::IsMoved); - if (::sus::mem::replace(state_, ResultState::IsMoved) == - ResultState::IsOk) { + if (storage_.is_ok()) { return ::sus::option::OptionIter( - Option(::sus::mem::take_and_destruct(::sus::marker::unsafe_fn, - storage_.ok_))); - } else { - storage_.destroy_err(); + Option(storage_.template take_ok())); + } else if (storage_.is_err()) { + storage_.drop_err(); return ::sus::option::OptionIter(Option()); + } else { + panic_with_message("Result used after move"); } } @@ -1027,35 +773,13 @@ class [[nodiscard]] Result final { friend constexpr bool operator==(const Result& l, const Result& r) noexcept requires(VoidOrEq && ::sus::cmp::Eq) { - ::sus::check(l.state_ != ResultState::IsMoved); - switch (l.state_) { - case ResultState::IsOk: - if constexpr (!std::is_void_v) - return r.is_ok() && l.storage_.ok_ == r.storage_.ok_; - else - return r.is_ok(); - case ResultState::IsErr: - return r.is_err() && l.storage_.err_ == r.storage_.err_; - case ResultState::IsMoved: break; - } - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); + return eq(l, r); } template requires(VoidOrEq && ::sus::cmp::Eq) friend constexpr bool operator==(const Result& l, const Result& r) noexcept { - ::sus::check(l.state_ != ResultState::IsMoved); - switch (l.state_) { - case ResultState::IsOk: - if constexpr (!std::is_void_v) - return r.is_ok() && l.storage_.ok_ == r.storage_.ok_; - else - return r.is_ok(); - case ResultState::IsErr: - return r.is_err() && l.storage_.err_ == r.storage_.err_; - case ResultState::IsMoved: break; - } - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); + return eq(l, r); } template requires(!(VoidOrEq && ::sus::cmp::Eq)) @@ -1078,49 +802,13 @@ class [[nodiscard]] Result final { const Result& r) noexcept requires(VoidOrOrd && ::sus::cmp::StrongOrd) { - ::sus::check(l.state_ != ResultState::IsMoved); - switch (l.state_) { - case ResultState::IsOk: - if (r.is_ok()) { - if constexpr (!std::is_void_v) - return std::strong_order(l.storage_.ok_, r.storage_.ok_); - else - return std::strong_ordering::equivalent; - } else { - return std::strong_ordering::greater; - } - case ResultState::IsErr: - if (r.is_err()) - return std::strong_order(l.storage_.err_, r.storage_.err_); - else - return std::strong_ordering::less; - case ResultState::IsMoved: break; - } - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); + return cmp(l, r); } template requires(VoidOrOrd && ::sus::cmp::StrongOrd) friend constexpr std::strong_ordering operator<=>( const Result& l, const Result& r) noexcept { - ::sus::check(l.state_ != ResultState::IsMoved); - switch (l.state_) { - case ResultState::IsOk: - if (r.is_ok()) { - if constexpr (!std::is_void_v) - return std::strong_order(l.storage_.ok_, r.storage_.ok_); - else - return std::strong_ordering::equivalent; - } else { - return std::strong_ordering::greater; - } - case ResultState::IsErr: - if (r.is_err()) - return std::strong_order(l.storage_.err_, r.storage_.err_); - else - return std::strong_ordering::less; - case ResultState::IsMoved: break; - } - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); + return cmp(l, r); } // sus::cmp::Ord> trait. @@ -1129,50 +817,14 @@ class [[nodiscard]] Result final { requires((!VoidOrOrd || !::sus::cmp::StrongOrd) && VoidOrWeakOrd && ::sus::cmp::Ord) { - ::sus::check(l.state_ != ResultState::IsMoved); - switch (l.state_) { - case ResultState::IsOk: - if (r.is_ok()) { - if constexpr (!std::is_void_v) - return std::weak_order(l.storage_.ok_, r.storage_.ok_); - else - return std::weak_ordering::equivalent; - } else { - return std::weak_ordering::greater; - } - case ResultState::IsErr: - if (r.is_err()) - return std::weak_order(l.storage_.err_, r.storage_.err_); - else - return std::weak_ordering::less; - case ResultState::IsMoved: break; - } - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); + return cmp(l, r); } template requires((!VoidOrOrd || !::sus::cmp::StrongOrd) && VoidOrWeakOrd && ::sus::cmp::Ord) friend constexpr std::weak_ordering operator<=>( const Result& l, const Result& r) noexcept { - ::sus::check(l.state_ != ResultState::IsMoved); - switch (l.state_) { - case ResultState::IsOk: - if (r.is_ok()) { - if constexpr (!std::is_void_v) - return std::weak_order(l.storage_.ok_, r.storage_.ok_); - else - return std::weak_ordering::equivalent; - } else { - return std::weak_ordering::greater; - } - case ResultState::IsErr: - if (r.is_err()) - return std::weak_order(l.storage_.err_, r.storage_.err_); - else - return std::weak_ordering::less; - case ResultState::IsMoved: break; - } - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); + return cmp(l, r); } // sus::cmp::PartialOrd> trait. @@ -1181,50 +833,14 @@ class [[nodiscard]] Result final { requires((!VoidOrWeakOrd || !::sus::cmp::Ord) && VoidOrPartialOrd && ::sus::cmp::PartialOrd) { - ::sus::check(l.state_ != ResultState::IsMoved); - switch (l.state_) { - case ResultState::IsOk: - if (r.is_ok()) { - if constexpr (!std::is_void_v) - return std::partial_order(l.storage_.ok_, r.storage_.ok_); - else - return std::partial_ordering::equivalent; - } else { - return std::partial_ordering::greater; - } - case ResultState::IsErr: - if (r.is_err()) - return std::partial_order(l.storage_.err_, r.storage_.err_); - else - return std::partial_ordering::less; - case ResultState::IsMoved: break; - } - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); + return cmp(l, r); } template requires((!VoidOrWeakOrd || !::sus::cmp::Ord) && VoidOrPartialOrd && ::sus::cmp::PartialOrd) friend constexpr std::partial_ordering operator<=>( const Result& l, const Result& r) noexcept { - ::sus::check(l.state_ != ResultState::IsMoved); - switch (l.state_) { - case ResultState::IsOk: - if (r.is_ok()) { - if constexpr (!std::is_void_v) - return std::partial_order(l.storage_.ok_, r.storage_.ok_); - else - return std::partial_ordering::equivalent; - } else { - return std::partial_ordering::greater; - } - case ResultState::IsErr: - if (r.is_err()) - return std::partial_order(l.storage_.err_, r.storage_.err_); - else - return std::partial_ordering::less; - case ResultState::IsMoved: break; - } - ::sus::unreachable_unchecked(::sus::marker::unsafe_fn); + return cmp(l, r); } template @@ -1237,59 +853,85 @@ class [[nodiscard]] Result final { friend class Result; // Since `T` may be a reference or a value type, this constructs the correct - // storage from a `T` object or a `T&&` (which is received as `T&`). + // storage type for `T`. template - static constexpr inline decltype(auto) copy_ok_to_storage(const U& t) { + requires(!std::is_void_v) + static constexpr inline decltype(auto) make_ok_storage(U&& t) { if constexpr (std::is_reference_v) return StoragePointer(t); else - return t; + return ::sus::forward(t); } - // Since `T` may be a reference or a value type, this constructs the correct - // storage from a `T` object or a `T&&` (which is received as `T&`). - template - static constexpr inline decltype(auto) move_ok_to_storage(U&& t) { - if constexpr (std::is_reference_v) - return StoragePointer(t); - else - return ::sus::move(t); + + template + static constexpr bool eq(const Result& l, const Result& r) noexcept { + ::sus::check(!l.storage_.is_moved() && !r.storage_.is_moved()); + if (l.storage_.is_ok()) { + if constexpr (std::is_void_v) { + return r.storage_.is_ok(); + } else if (r.storage_.is_ok()) { + return l.storage_.template get_ok() == + r.storage_.template get_ok(); + } else { + return false; + } + } else { + if (r.storage_.is_err()) { + return l.storage_.get_err() == r.storage_.get_err(); + } else { + return false; + } + } + } + + template <::sus::cmp::Ordering O, class U, class F> + static constexpr O cmp(const Result& l, const Result& r) noexcept { + ::sus::check(!l.storage_.is_moved() && !r.storage_.is_moved()); + if (l.storage_.is_ok()) { + if (r.storage_.is_ok()) { + if constexpr (std::is_void_v) { + return O::equivalent; + } else { + return l.storage_.template get_ok() <=> + r.storage_.template get_ok(); + } + } else { + return O::greater; + } + } else { + if (r.storage_.is_err()) { + return l.storage_.get_err() <=> r.storage_.get_err(); + } else { + return O::less; + } + } } // Constructors for `Ok`. enum WithOk { WITH_OK }; constexpr inline Result(WithOk) noexcept requires(std::is_void_v) - : storage_(), state_(ResultState::IsOk) {} - template U> - constexpr inline Result(WithOk, const U& t) noexcept - : storage_(__private::kWithT, t), state_(ResultState::IsOk) {} - template U> - constexpr inline Result(WithOk, U&& t) noexcept - requires(::sus::mem::Move) - : storage_(__private::kWithT, ::sus::move(t)), - state_(ResultState::IsOk) {} + : storage_(__private::WITH_T) {} + template + constexpr inline Result(WithOk, U&& u) noexcept + requires(!std::is_void_v) + : storage_(__private::WITH_T, ::sus::forward(u)) {} // Constructors for `Err`. enum WithErr { WITH_ERR }; - constexpr inline Result(WithErr, const E& e) noexcept - : storage_(__private::kWithE, e), state_(ResultState::IsErr) {} - constexpr inline Result(WithErr, E&& e) noexcept - requires(::sus::mem::Move) - : storage_(__private::kWithE, ::sus::move(e)), - state_(ResultState::IsErr) {} + template + constexpr inline Result(WithErr, F&& f) noexcept + : storage_(__private::WITH_E, ::sus::forward(f)) {} - struct VoidStorage {}; using OkStorageType = - std::conditional_t, StoragePointer, - std::conditional_t, VoidStorage, T>>; - - [[sus_no_unique_address]] Storage storage_; - ResultState state_; - - sus_class_trivially_relocatable_if_types( - ::sus::marker::unsafe_fn, - std::remove_const_t>, - std::remove_const_t>, - decltype(state_)); + std::conditional_t, StoragePointer, T>; + using Storage = std::conditional_t< // + std::is_void_v, // + __private::StorageVoid, // + __private::StorageNonVoid>; + [[no_unique_address]] Storage storage_; + + sus_class_trivially_relocatable_if_types(::sus::marker::unsafe_fn, + decltype(storage_)); }; /// Implicit for-ranged loop iteration via [`Result::iter()`]( diff --git a/sus/result/result_unittest.cc b/sus/result/result_unittest.cc index 4a546b936..68ac25dae 100644 --- a/sus/result/result_unittest.cc +++ b/sus/result/result_unittest.cc @@ -59,9 +59,31 @@ struct TailPadding { i32 j; // 4 bytes of tail padding, which the Result can use for its own state. }; +// The Result's state can go into the tail padding of T and E. // MSVC doesn't take advantage of `[[no_unique_address]]`. static_assert(sizeof(Result) == sizeof(TailPadding) + sus_if_msvc_else(8, 0)); +static_assert(sizeof(Result) == + sizeof(TailPadding) + sus_if_msvc_else(8, 0)); + +template +struct Holds { + [[sus_no_unique_address]] T t; + char c = 0; +}; +// An element in the Result's union has tail padding, so it can't allow anything +// outside the Result from ending up inside there. +static_assert(sizeof(Holds>) == + sizeof(Result) + 8); +static_assert(sizeof(Holds>) == + sizeof(Result) + 8); +// Nothing in the Result's union has tail padding, the tail padding of the +// Result itself can be used externally. +// MSVC doesn't take advantage of `[[no_unique_address]]`. +static_assert(sizeof(Holds>) == + sizeof(Result) + sus_if_msvc_else(8, 0)); +static_assert(sizeof(Holds>) == + sizeof(Result) + sus_if_msvc_else(8, 0)); struct Error {}; @@ -576,6 +598,7 @@ TEST(Result, Expect) { TEST(ResultDeathTest, Unwrap) { #if GTEST_HAS_DEATH_TEST + struct Foo {}; // clang-tidy does weird things here without this. { auto r = Result::with_err(Error()); EXPECT_DEATH(r.as_value(), "PANIC! at 'Result has error state'"); diff --git a/sus/test/behaviour_types.h b/sus/test/behaviour_types.h index 9f2251512..59a5c7d9c 100644 --- a/sus/test/behaviour_types.h +++ b/sus/test/behaviour_types.h @@ -68,7 +68,7 @@ struct TriviallyMoveableNotDestructible final { TriviallyMoveableNotDestructible&&) = default; constexpr TriviallyMoveableNotDestructible& operator=( TriviallyMoveableNotDestructible&&) = default; - constexpr ~TriviallyMoveableNotDestructible(){}; + constexpr ~TriviallyMoveableNotDestructible() {} constexpr TriviallyMoveableNotDestructible(int i) : i(i) {} int i; };