Repository: SerenityOS/serenity
File: AK/Optional.h (C/C++)

Code before:
        if (this == &other)
            return *this;

        clear();
        m_has_value = other.m_has_value;
        if (other.has_value())
            construct_at<RemoveConst<T>>(&m_storage, other.value());
        return *this;
    }

    constexpr Optional(Optional const& other) = default;
    constexpr Optional& operator=(Optional const&) = default;

    Optional(Optional&& other)
    requires(!IsMoveConstructible<T>)
    = delete;
    ALWAYS_INLINE constexpr Optional(Optional&& other)
    requires(IsMoveConstructible<T> && !IsTriviallyMoveConstructible<T>)
        : m_has_value(other.m_has_value)
    {
        if (other.has_value())
            construct_at<RemoveConst<T>>(&m_storage, other.release_value());
    }

    // Note: The MoveConstructible only versions are to allow for non-move assignable types,
    //       Such as types containing a reference, as move assigning them into an Optional
    //       still makes sense

    Optional& operator=(Optional&& other)
    requires(!IsMoveConstructible<T> || !IsDestructible<T>)
    = delete;
    ALWAYS_INLINE constexpr Optional& operator=(Optional&& other)
    requires(IsMoveAssignable<T> && IsMoveConstructible<T> && !IsTriviallyMoveAssignable<T>)
    {
        if (this == &other)
            return *this;

        if (m_has_value && other.m_has_value) {
            value() = other.release_value();
        } else if (m_has_value) {
            value().~T();
            m_has_value = false;
        } else if (other.m_has_value) {
            construct_at<RemoveConst<T>>(&m_storage, other.release_value());
            m_has_value = true;
        }
        return *this;
    }

    ALWAYS_INLINE constexpr Optional& operator=(Optional&& other)
    requires(IsMoveConstructible<T> && !IsMoveAssignable<T>)

Review comment (refactor):
nit: There is an uppercase after a comma, and the sentence is missing a final dot.
And you might want to split this long sentence into two.

While I'm here, I already liked this `constexpr` `Optional` but there is a side effect that I really appreciate too: gdb is now capable of correctly interpreting the inner type. Thanks for your work!

Code after:
        if (this == &other)
            return *this;

        clear();
        m_has_value = other.m_has_value;
        if (other.has_value())
            construct_at<RemoveConst<T>>(&m_storage, other.value());
        return *this;
    }

    constexpr Optional(Optional const& other) = default;
    constexpr Optional& operator=(Optional const&) = default;

    Optional(Optional&& other)
    requires(!IsMoveConstructible<T>)
    = delete;
    ALWAYS_INLINE constexpr Optional(Optional&& other)
    requires(IsMoveConstructible<T> && !IsTriviallyMoveConstructible<T>)
        : m_has_value(other.m_has_value)
    {
        if (other.has_value())
            construct_at<RemoveConst<T>>(&m_storage, other.release_value());
    }

    // Note: The MoveConstructible only versions are to allow for non-move assignable types,
    //       such as types containing a reference.
    //       Move assigning into an Optional still makes sense in these cases,
    //       as to replace its contents or to allow for find-like patterns through iterator helpers.

    Optional& operator=(Optional&& other)
    requires(!IsMoveConstructible<T> || !IsDestructible<T>)
    = delete;
    ALWAYS_INLINE constexpr Optional& operator=(Optional&& other)
    requires(
        IsMoveAssignable<T> && IsMoveConstructible<T>
        && (!IsTriviallyMoveAssignable<T> || !IsTriviallyMoveConstructible<T> || !IsTriviallyDestructible<T>))
    {
        if (this == &other)
            return *this;

        if (m_has_value && other.m_has_value) {
            value() = other.release_value();
        } else if (m_has_value) {
            value().~T();
            m_has_value = false;
        } else if (other.m_has_value) {
            construct_at<RemoveConst<T>>(&m_storage, other.release_value());
            m_has_value = true;
        }
        return *this;
    }