HomePhabricator

Introduce generic 'Result' class

Description

Introduce generic 'Result' class

Summary:

Useful to encapsulate the function result object (in case of having it) or, in case of failure, the failure reason.

This let us clean lot of boilerplate code, as now instead of returning a boolean and having to add a ref arg for the
return object and another ref for the error string. We can simply return a 'BResult<Obj>'.

Example of what we currently have:

bool doSomething(arg1, arg2, arg3, arg4, &result, &error_string) {
    do something...
    if (error) {
        error_string = "something bad happened";
        return false;
    }

    result = goodResult;
    return true;
}

Example of what we will get with this commit:

BResult<Obj> doSomething(arg1, arg2, arg3, arg4) {
    do something...
    if (error) return {"something happened"};

    // good
    return {goodResult};
}

This allows a similar boilerplate cleanup on the function callers side as well. They don't have to add the extra
pre-function-call error string and result object declarations to pass the references to the function.

https://github.com/bitcoin/bitcoin/pull/25218/commits/7a45c33d1f8a758850cf8e7bd6ad508939ba5c0d


Prepare BResult for non-copyable types

https://github.com/bitcoin/bitcoin/pull/25594/commits/fa8de09edc9ec4e6d171df80f746174a0ec58afb


Refactor: Replace BResult with util::Result

Rename BResult class to util::Result and update the class interface to be
more compatible with std::optional and with a full-featured result class
implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for
this change is to update existing BResult usages now so they don't have to
change later when more features are added in #25665.

This change makes the following improvements originally implemented in #25665:

  • More explicit API. Drops potentially misleading BResult constructor that treats any bilingual string argument as an error. Adds util::Error constructor so it is never ambiguous when a result is being assigned an error or non-error value.
  • Better type compatibility. Supports util::Result<bilingual_str> return values to hold translated messages which are not errors.
  • More standard and consistent API. util::Result supports most of the same operators and methods as std::optional. BResult had a less familiar interface with HasRes/GetObj/ReleaseObj methods. The Result/Res/Obj naming was also not internally consistent.
  • Better code organization. Puts src/util/ code in the util:: namespace so naming reflects code organization and it is obvious where the class is coming from. Drops "B" from name because it is undocumented what it stands for (bilingual?)
  • Has unit tests.

https://github.com/bitcoin/bitcoin/pull/25721/commits/a23cca56c0a7f4a267915b4beba3af3454c51603


util: Add void support to util::Result

A minimal (but hacky) way to add support for void to Result
originally posted https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1195604095

https://github.com/bitcoin/bitcoin/pull/25977/commits/5f49cb1bc8e6ba0671c21bf6292d2d3de43fd001


This is a partial backport of core#25218, core#25594, core#25721 and core#25977

The wallet use cases in the first 3 pull requests are not yet applicable to Bitcoin ABC due to missing dependencies.
The std::optional<bilingual_str> use cases from core#25977 will be backported in their own diff.

Test Plan: ninja && ninja check

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14991

Details

Provenance
furszy <matiasfurszyfer@protonmail.com>Authored on Apr 8 2022, 19:24
PiRKCommitted on Dec 15 2023, 11:33
PiRKPushed on Dec 15 2023, 11:34
Reviewer
Restricted Project
Differential Revision
D14991: Introduce generic 'Result' class
Parents
rABC716ae6a36889: [electrum] move checkpoint data to a json file
Branches
Unknown
Tags
Unknown

Event Timeline