Repository: bitcoin/bitcoin
File: src/util/result.h (C/C++)

Code before:
// Copyright (c) 2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or https://www.opensource.org/licenses/mit-license.php.

#ifndef BITCOIN_UTIL_RESULT_H
#define BITCOIN_UTIL_RESULT_H

#include <attributes.h>
#include <util/translation.h>

#include <variant>

namespace util {

struct Error {
    bilingual_str message;
};

//! The util::Result class provides a standard way for functions to return
//! either error messages or result values.
//!
//! It is intended for high-level functions that need to report error strings to
//! end users. Lower-level functions that don't need this error-reporting and
//! only need error-handling should avoid util::Result and instead use standard
//! classes like std::optional, std::variant, and std::tuple, or custom structs
//! and enum types to return function results.
//!
//! Usage examples can be found in \example ../test/result_tests.cpp, but in
//! general code returning `util::Result<T>` values is very similar to code
//! returning `std::optional<T>` values. Existing functions returning
//! `std::optional<T>` can be updated to return `util::Result<T>` and return
//! error strings usually just replacing `return std::nullopt;` with `return
//! util::Error{error_string};`.
template <class M>
class Result
{
private:
    using T = std::conditional_t<std::is_same_v<M, void>, std::monostate, M>;

    std::variant<bilingual_str, T> m_variant;

    //! Disallow copy constructor, require Result to be moved for efficiency.
    Result(const Result&) = delete;

    //! Disallow operator= to avoid confusion in the future when the Result
    //! class gains support for richer error reporting, and callers should have
    //! ability to set a new result value without clearing existing error

Review comment (suggestion):
I think the current wording is a bit vague. It might be rewritten along the lines of:
```
//! It is intended for high-level functions that need to report error strings **directly**
//! to end users. Any lower-level functions that don't need this error-reporting...

```
However, Result in our codebase isn’t limited to reporting errors to end users. As @ryanofsky noted in PR #25665, there are broader use cases. One strong example is:
>Result may also be a better choice than std::expected when returning pointer values. (Followup #26022 adds a ResultPtr specialization that avoids double dereferencing.)

I think these considerations should also be part of the discussion for this PR, and we should agree on the responsibilities of each type. Consequently, the documentation should be updated to reflect these broader use cases more accurately.

Code after:
// Copyright (c) 2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or https://www.opensource.org/licenses/mit-license.php.

#ifndef BITCOIN_UTIL_RESULT_H
#define BITCOIN_UTIL_RESULT_H

#include <attributes.h>
#include <util/translation.h>

#include <variant>

namespace util {

struct Error {
    bilingual_str message;
};

//! The util::Result class provides a standard way for functions to return
//! either error messages or result values.
//!
//! It is intended for high-level functions that need to report error strings to
//! end users. Lower-level functions that don't need this error-reporting and
//! that only need error-handling should avoid util::Result and instead use
//! util::Expected, std::optional, std::variant, or custom structs
//! and enum types to return function results.
//!
//! Usage examples can be found in \example ../test/result_tests.cpp, but in
//! general code returning `util::Result<T>` values is very similar to code
//! returning `std::optional<T>` values. Existing functions returning
//! `std::optional<T>` can be updated to return `util::Result<T>` and return
//! error strings usually just replacing `return std::nullopt;` with `return
//! util::Error{error_string};`.
template <class M>
class Result
{
private:
    using T = std::conditional_t<std::is_same_v<M, void>, std::monostate, M>;

    std::variant<bilingual_str, T> m_variant;

    //! Disallow copy constructor, require Result to be moved for efficiency.
    Result(const Result&) = delete;

    //! Disallow operator= to avoid confusion in the future when the Result
    //! class gains support for richer error reporting, and callers should have
    //! ability to set a new result value without clearing existing error