system/serenity
system/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2024-07-21 18:15:58 +00:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

AK: Change ErrorOr to contain a Variant rather than inherit from it

Browse source 
GCC seems to get tripped up over this inheritance when converting from
an ErrorOr<StringView> to the partially specialized ErrorOr<void>. See
the following snippet:

    NEVER_INLINE ErrorOr<StringView> foo()
    {
        auto string = "abc"sv;
        outln("{:p}", string.characters_without_null_termination());
        return string;
    }

    NEVER_INLINE ErrorOr<void> bar()
    {
        auto string = TRY(foo());
        outln("{:p}", string.characters_without_null_termination());

        VERIFY(!string.starts_with('#'));
        return {};
    }

    int main()
    {
        MUST(bar());
    }

On some machines, bar() will contain a StringView whose pointer has had
its upper bits set to 0:

    0x000000010cafd6f8
    0x000000000cafd6f8

I'm not 100% clear on what's happening in the default-generated Variant
destructor that causes this. Probably worth investigating further.

The error would also be alleviated by making the Variant destructor
virtual, but rather than that, let's make ErrorOr simply contain a
Variant rather than inherit from it.

Fixes #15449.
This commit is contained in:
Timothy Flynn 2022-10-07 08:52:47 -04:00 committed by Linus Groh
parent f38c68177b
commit e897008449
1 changed files with 17 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

59
AK/Error.h
View file

@ -6,7 +6,6 @@
#pragma once
#include <AK/Optional.h>
#include <AK/StringView.h>
#include <AK/Try.h>
#include <AK/Variant.h>
@ -74,32 +73,36 @@ private:
};
template<typename T, typename ErrorType>
class [[nodiscard]] ErrorOr final : public Variant<T, ErrorType> {
class [[nodiscard]] ErrorOr {
public:
using Variant<T, ErrorType>::Variant;
ErrorOr() requires(IsSame<T, Empty>)
: m_value_or_error(Empty {})
{
}
template<typename U>
ALWAYS_INLINE ErrorOr(U&& value) requires(!IsSame<RemoveCVReference<U>, ErrorOr<T>>)
: Variant<T, ErrorType>(forward<U>(value))
ALWAYS_INLINE ErrorOr(U&& value) requires(!IsSame<RemoveCVReference<U>, ErrorOr<T, ErrorType>>)
: m_value_or_error(forward<U>(value))
{
}
#ifdef __serenity__
ErrorOr(ErrnoCode code)
: Variant<T, ErrorType>(Error::from_errno(code))
: m_value_or_error(Error::from_errno(code))
{
}
#endif
T& value()
{
return this->template get<T>();
return m_value_or_error.template get<T>();
}
T const& value() const { return this->template get<T>(); }
ErrorType& error() { return this->template get<ErrorType>(); }
ErrorType const& error() const { return this->template get<ErrorType>(); }
T const& value() const { return m_value_or_error.template get<T>(); }
bool is_error() const { return this->template has<ErrorType>(); }
ErrorType& error() { return m_value_or_error.template get<ErrorType>(); }
ErrorType const& error() const { return m_value_or_error.template get<ErrorType>(); }
bool is_error() const { return m_value_or_error.template has<ErrorType>(); }
T release_value() { return move(value()); }
ErrorType release_error() { return move(error()); }
@ -111,41 +114,13 @@ public:
}
private:
// 'downcast' is fishy in this context. Let's hide it by making it private.
using Variant<T, ErrorType>::downcast;
Variant<T, ErrorType> m_value_or_error;
};
// Partial specialization for void value type
template<typename ErrorType>
class [[nodiscard]] ErrorOr<void, ErrorType> {
class [[nodiscard]] ErrorOr<void, ErrorType> : public ErrorOr<Empty, ErrorType> {
public:
ErrorOr(ErrorType error)
: m_error(move(error))
{
}
#ifdef __serenity__
ErrorOr(ErrnoCode code)
: m_error(Error::from_errno(code))
{
}
#endif
ErrorOr() = default;
ErrorOr(ErrorOr&& other) = default;
ErrorOr(ErrorOr const& other) = default;
~ErrorOr() = default;
ErrorOr& operator=(ErrorOr&& other) = default;
ErrorOr& operator=(ErrorOr const& other) = default;
ErrorType const& error() const { return m_error.value(); }
bool is_error() const { return m_error.has_value(); }
ErrorType release_error() { return m_error.release_value(); }
void release_value() { }
private:
Optional<ErrorType> m_error;
using ErrorOr<Empty, ErrorType>::ErrorOr;
};
}