Unwrap underlying error value without runtime checks

[oracle-9]

Would it be possible to implement a member function that returns the result's underlying error, similar to .error(), but without without the runtime check? It would be analogous to operator*().

Additionally, this member function wouldn't need the error type to be default constructible, since it would never need to construct one. This would in turn allow non-default constructible errors to be used in conjunction with result.

Dummy implementation:

// Extracts error by move.
template <typename T, typename E>
auto constexpr result<T, E>::operator*() && noexcept -> typename std::add_rvalue_reference_t<T> {
    return std::move(m_storage.storage.m_error);
}

// Extracts error by copy.
template <typename T, typename E>
auto constexpr result<T, E>::operator*() const& noexcept -> typename std::add_lvalue_reference_t<typename std::add_const_t<T>>
{
  return m_storage.storage.m_value;
}

Finally, I noticed that the result's member functions that move the underlying value or error are unconditionally noexcept. However, this isn't always the case. The following is valid code:

struct ThrowsOnMove {
    ThrowsOnMove() = default;
    ThrowsOnMove(ThrowsOnMove&&) {
        throw 42;
    }
};

cpp::fail(ThrowsOnMove()).error(); // will call std::terminate.

This could be solved by using the noexcept operator, e.g.:

template <typename E>
auto constexpr failure<E>::error() &&
    noexcept(std::is_nothrow_move_constructible_v<E>)
    -> typename std::add_rvalue_reference_t<E>
{
    return std::move(m_failure);
}

[bitwizeshift]

Hey, sorry taking a few days to get to this. Thank you for the feedback!

Are you able to describe the type of situation you're currently working in where this would be needed? Is there any way to perform indirection to make the error type lightweight?

Getting a reference to the underlying operator has been requested before -- I'm not sure if you saw this already (#5).
I'm not against potentially adding a mechanism to do this -- although this does go against the initial design principles I was attempting to uphold. In particular, I designed this implementation with the idea that error types indicate more of a "status", and are intended to be "lightweight". I also really wanted to avoid the case where calling error() (or some equivalent function) might now throw an exception, or cause UB -- since I would rather limit the scope of how easy it is to create mistakes using this utility.

In most practical cases I've worked with, errors can be represented by easy enum types, which make this simple. In the more complex cases, such as distributing things like stack-traces, contextual information, etc, this could be done by throwing it into a structure pointed to by a unique_ptr or a shared_ptr. Since such error cases that generate such complex data should be rare and uncommon, this is generally not a pessimization to use such an indirection. Would an approach like this work for your circumstance?

For example:

struct large_error_context {
public:
  large_error_context(); // default constructs: assumes no error
  large_error_context(...); // creates with error
private:
    struct info { // some struct of large data
        stack_trace trace;
        source_location location;
        other_large_data data;
    };
    std::unique_ptr<info> m_info;
};

Finally, I noticed that the result's member functions that move the underlying value or error are unconditionally noexcept. However, this isn't always the case. The following is valid code:

 struct ThrowsOnMove {
    ThrowsOnMove() = default;
    ThrowsOnMove(ThrowsOnMove&&) {
        throw 42;
    }
};

cpp::fail(ThrowsOnMove()).error(); // will call std::terminate.

Can you point to some (other) specific lines where this creates an issue?

Your example of cpp::fail(ThrowsOnMove()).error() doesn't cause such an issue because the error() function does not invoke any constructors. The fail function itself will, however these functions use proper detection and resolve before error is ever called (which is why in the first link, we're able to catch the exception). The failure<E>::error() function in question only returns a reference, and a cast to a reference doesn't create any constructor calls -- they are no-ops that only affect the type system. This makes these functions always non-throwing.

[oracle-9]

I just realized that in my post the dummy implementations are returning T instead of E, and using operator* which is already being used to unwrap the underlying T.
My bad, I hope it still got the point across.

Hey, sorry taking a few days to get to this. Thank you for the feedback!

No problem :)

Are you able to describe the type of situation you're currently working in where this would be needed?

Sure. A bunch of my code is littered with the following pattern:

enum class Error { /* ... */ };

struct T {
    // fields omitted.
};

struct U {
    T t;
    // other fields omitted.
};

result<T, Error> f();

result<U, Error> g() {
    auto r = f();

    if (r.has_error()) {
        // propagate error.
        return cpp::fail(std::move(r).error());
        // calling error() does a check even though I've already done it.
    }

    T& t = *r; // I can call operator* because I already checked before.

    // do things with t.
}

It would be nice if I could replace

        return cpp::fail(std::move(r).error());

with something like

        return cpp::fail(std::move(r).error_unchecked());

Another issue I found with error() is the requirement it imposes on the error type: it must be default constructible.
Example:

struct Error {
    void* context; // doesn't need to be void, just some pointer to borrowed memory.

    Error() = delete; // Prevent the construction of errors with nullptr.

    Error(void* context) noexcept : context(context) {}
};

// Assume the existence of T, U and f() from the previous example. 

result<T, Error> g() {
    auto r = f();

    if (r.has_error()) {
        // propagate error.
        return cpp::fail(std::move(r).error());
        // will not compile because Error is not default constructible, even
        // though it shouldn't be required in this context.
    }
}

Getting a reference to the underlying operator has been requested before -- I'm not sure if you saw this already (#5).

Yes, I had read that post.
However, in this case, the error type is already small, it's the size of a pointer or, in the first case, an enum.
Wrapping it in a smart pointer would add unnecessary complexity to the program.
The real issue isn't the size of the error, it's semantics. The error type may need some parameters in order to be constructed, and default construction just doesn't make sense in those cases.

I also really wanted to avoid the case where calling error() (or some equivalent function) might now throw an exception, or cause UB

I would also find it surprising behavior if it did that.
Still, I think adding (not replacing) an error function with a very explicit name error_unchecked or assume_error or something similar would convey the unsafeness of the operation. This function would return the error value without performing any checks, throwing any exceptions or default constructing the error:

// result&& -> error&&
template <typename T, typename E>
auto constexpr result<T, E>::assume_error() && noexcept -> typename std::add_rvalue_reference_t<E> {
    return std::move(m_storage.storage.m_error);
}

// result const& -> error const&
template <typename T, typename E>
auto constexpr result<T, E>::assume_error() const& noexcept -> typename std::add_lvalue_reference_t<typename std::add_const_t<E>>
{
    return m_storage.storage.m_error;
}

It would solve the unnecessary check in the first example, and the default constructible requirement from the second example.

Your example of cpp::fail(ThrowsOnMove()).error() doesn't cause such an issue because the error() function does not invoke any constructors

You're right, my bad. I've since been playing around with those explosive types and figured it wasn't an issue.