Skip to content

Commit

Permalink
Add util::ResultPtr class
Browse files Browse the repository at this point in the history
The util::ResultPtr class is a wrapper for util::Result providing syntax sugar
to make it less awkward to use with returned pointers. Instead of needing to be
deferenced twice with **result or (*result)->member, it only needs to be
dereferenced once with *result or result->member. Also when it is used in a
boolean context, instead of evaluating to true when there is no error and the
pointer is null, it evaluates to false so it is straightforward to determine
whether the result points to something.

This PR is only partial a solution to bitcoin#26004 because while it makes it easier
to check for null pointer values, it does not make it impossible to return a
null pointer value inside a successful result. It would be possible to enforce
that successful results always contain non-null pointers by using a not_null
type (bitcoin#24423) in combination with
ResultPtr, though.
  • Loading branch information
ryanofsky committed Jun 17, 2024
1 parent 256987e commit 40d119f
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 0 deletions.
35 changes: 35 additions & 0 deletions src/test/result_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,42 @@ util::Result<std::unique_ptr<NoCopyNoMove>> DerivedToBaseFn(util::Result<std::un

BOOST_AUTO_TEST_CASE(derived_to_base)
{
// Check derived to base conversions work for util::Result
BOOST_CHECK_EQUAL(*Assert(*Assert(DerivedToBaseFn(std::make_unique<Derived>(5)))), 5);

// Check conversions work between util::Result and util::ResultPtr
util::ResultPtr<std::unique_ptr<Derived>> derived{std::make_unique<Derived>(5)};
util::ResultPtr<std::unique_ptr<NoCopyNoMove>> base{DerivedToBaseFn(std::move(derived))};
BOOST_CHECK_EQUAL(*Assert(base), 5);
}

//! For testing ResultPtr, return pointer to a pair of ints, or return pointer to null, or return an error message.
util::ResultPtr<std::unique_ptr<std::pair<int, int>>> PtrFn(std::optional<std::pair<int, int>> i, bool success)
{
if (success) return i ? std::make_unique<std::pair<int, int>>(*i) : nullptr;
return util::Error{strprintf(Untranslated("PtrFn(%s) error."), i ? strprintf("%i, %i", i->first, i->second) : "nullopt")};
}

BOOST_AUTO_TEST_CASE(check_ptr)
{
auto result_pair = PtrFn(std::pair{1, 2}, true);
ExpectResult(result_pair, true, {});
BOOST_CHECK(result_pair);
BOOST_CHECK_EQUAL(result_pair->first, 1);
BOOST_CHECK_EQUAL(result_pair->second, 2);
BOOST_CHECK(*result_pair == std::pair(1,2));

auto result_null = PtrFn(std::nullopt, true);
ExpectResult(result_null, true, {});
BOOST_CHECK(!result_null);

auto result_error_pair = PtrFn(std::pair{1, 2}, false);
ExpectResult(result_error_pair, false, Untranslated("PtrFn(1, 2) error."));
BOOST_CHECK(!result_error_pair);

auto result_error_null = PtrFn(std::nullopt, false);
ExpectResult(result_error_null, false, Untranslated("PtrFn(nullopt) error."));
BOOST_CHECK(!result_error_null);
}

BOOST_AUTO_TEST_SUITE_END()
27 changes: 27 additions & 0 deletions src/util/result.h
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,33 @@ decltype(auto) operator>>(SrcResult&& src LIFETIMEBOUND, DstResult&& dst)
return static_cast<SrcType&&>(src);
}

//! Wrapper around util::Result that is less awkward to use with pointer types.
//!
//! It overloads pointer and bool operators so it isn't necessary to dereference
//! the result object twice to access the result value, so it possible to call
//! methods with `result->Method()` rather than `(*result)->Method()` and check
//! whether the pointer is null with `if (result)` rather than `if (result &&
//! *result)`.
//!
//! The `ResultPtr` class just adds syntax sugar to `Result` class. It is still
//! possible to access the result pointer directly using `ResultPtr` `value()`
//! and `has_value()` methods.
template <typename T, typename F = void>
class ResultPtr : public Result<T, F>
{
public:
// Inherit Result constructors (excluding copy and move constructors).
using Result<T, F>::Result;

// Inherit Result copy and move constructors.
template<typename O>
ResultPtr(O&& other) : Result<T, F>{std::forward<O>(other)} {}

explicit operator bool() const noexcept { return this->has_value() && this->value(); }
auto* operator->() const { assert(this->value()); return &*this->value(); }
auto& operator*() const { assert(this->value()); return *this->value(); }
};

//! Join error and warning messages in a space separated string. This is
//! intended for simple applications where there's probably only one error or
//! warning message to report, but multiple messages should not be lost if they
Expand Down

0 comments on commit 40d119f

Please sign in to comment.