diff --git a/src/Makefile.am b/src/Makefile.am index 8905c0ad1cd25..7702e6474e471 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -747,6 +747,7 @@ libbitcoin_util_a_SOURCES = \ util/moneystr.cpp \ util/rbf.cpp \ util/readwritefile.cpp \ + util/result.cpp \ util/signalinterrupt.cpp \ util/thread.cpp \ util/threadinterrupt.cpp \ @@ -986,6 +987,7 @@ libbitcoinkernel_la_SOURCES = \ util/hasher.cpp \ util/moneystr.cpp \ util/rbf.cpp \ + util/result.cpp \ util/serfloat.cpp \ util/signalinterrupt.cpp \ util/strencodings.cpp \ diff --git a/src/test/result_tests.cpp b/src/test/result_tests.cpp index c3fd7f50944c8..84e02dfc9c3de 100644 --- a/src/test/result_tests.cpp +++ b/src/test/result_tests.cpp @@ -2,10 +2,17 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include +#include +#include #include +#include +#include +#include +#include inline bool operator==(const bilingual_str& a, const bilingual_str& b) { @@ -90,15 +97,52 @@ enum FnError { ERR1, ERR2 }; util::Result IntFailFn(int i, bool success) { - if (success) return i; + if (success) return {util::Warning{Untranslated(strprintf("int %i warn.", i))}, i}; return {util::Error{Untranslated(strprintf("int %i error.", i))}, i % 2 ? ERR1 : ERR2}; } +util::Result StrFailFn(int i, bool success) +{ + auto result = IntFailFn(i, success); + if (!success) return {util::MoveMessages(result), util::Error{Untranslated("str error")}, result.GetFailure()}; + return {util::MoveMessages(result), strprintf("%i", *result)}; +} + util::Result EnumFailFn(FnError ret) { return {util::Error{Untranslated("enum fail.")}, ret}; } +util::Result WarnFn() +{ + return {util::Warning{Untranslated("warn.")}}; +} + +util::Result MultiWarnFn(int ret) +{ + util::Result result; + for (int i = 0; i < ret; ++i) { + result.AddWarning(strprintf(Untranslated("warn %i."), i)); + } + return {util::MoveMessages(result), ret}; +} + +util::Result ChainedFailFn(FnError arg, int ret) +{ + return {util::Error{Untranslated("chained fail.")}, util::MoveMessages(EnumFailFn(arg)), util::MoveMessages(WarnFn()), ret}; +} + +util::Result AccumulateFn(bool success) +{ + util::Result result; + util::Result x = MultiWarnFn(1) >> result; + BOOST_REQUIRE(x); + util::Result y = MultiWarnFn(2) >> result; + BOOST_REQUIRE(y); + result.Set(IntFailFn(*x + *y, success)); + return result; +} + util::Result TruthyFalsyFn(int i, bool success) { if (success) return i; @@ -140,7 +184,13 @@ BOOST_AUTO_TEST_CASE(check_returned) ExpectFail(NoCopyNoMoveFn(5, false), Untranslated("nocopynomove 5 error."), 5); ExpectSuccess(StrFn(Untranslated("S"), true), {}, Untranslated("S")); ExpectResult(StrFn(Untranslated("S"), false), false, Untranslated("str S error.")); + ExpectSuccess(StrFailFn(1, true), Untranslated("int 1 warn."), "1"); + ExpectFail(StrFailFn(2, false), Untranslated("int 2 error. str error"), ERR2); ExpectFail(EnumFailFn(ERR2), Untranslated("enum fail."), ERR2); + ExpectFail(ChainedFailFn(ERR1, 5), Untranslated("chained fail. enum fail. warn."), 5); + ExpectSuccess(MultiWarnFn(3), Untranslated("warn 0. warn 1. warn 2."), 3); + ExpectSuccess(AccumulateFn(true), Untranslated("warn 0. warn 0. warn 1. int 3 warn."), 3); + ExpectFail(AccumulateFn(false), Untranslated("int 3 error. warn 0. warn 0. warn 1."), ERR1); ExpectSuccess(TruthyFalsyFn(0, true), {}, 0); ExpectFail(TruthyFalsyFn(0, false), Untranslated("failure value 0."), 0); ExpectSuccess(TruthyFalsyFn(1, true), {}, 1); @@ -156,7 +206,7 @@ BOOST_AUTO_TEST_CASE(check_set) result.Set({util::Error{Untranslated("error")}, ERR1}); ExpectFail(result, Untranslated("error"), ERR1); result.Set(2); - ExpectSuccess(result, Untranslated(""), 2); + ExpectSuccess(result, Untranslated("error"), 2); // Test the same thing but with non-copyable success and failure types. util::Result result2{0}; @@ -164,7 +214,7 @@ BOOST_AUTO_TEST_CASE(check_set) result2.Set({util::Error{Untranslated("error")}, 3}); ExpectFail(result2, Untranslated("error"), 3); result2.Set(4); - ExpectSuccess(result2, Untranslated(""), 4); + ExpectSuccess(result2, Untranslated("error"), 4); } BOOST_AUTO_TEST_CASE(check_dereference_operators) @@ -188,6 +238,16 @@ BOOST_AUTO_TEST_CASE(check_value_or) BOOST_CHECK_EQUAL(StrFn(Untranslated("A"), false).value_or(Untranslated("B")), Untranslated("B")); } +BOOST_AUTO_TEST_CASE(check_message_accessors) +{ + util::Result result{util::Error{Untranslated("Error.")}, util::Warning{Untranslated("Warning.")}}; + BOOST_CHECK_EQUAL(result.GetErrors().size(), 1); + BOOST_CHECK_EQUAL(result.GetErrors()[0], Untranslated("Error.")); + BOOST_CHECK_EQUAL(result.GetWarnings().size(), 1); + BOOST_CHECK_EQUAL(result.GetWarnings()[0], Untranslated("Warning.")); + BOOST_CHECK_EQUAL(util::ErrorString(result), Untranslated("Error. Warning.")); +} + struct Derived : NoCopyNoMove { using NoCopyNoMove::NoCopyNoMove; }; diff --git a/src/util/result.cpp b/src/util/result.cpp new file mode 100644 index 0000000000000..e1b7eb213e420 --- /dev/null +++ b/src/util/result.cpp @@ -0,0 +1,32 @@ +// 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. + +#include + +#include +#include +#include +#include + +namespace util { +namespace detail { +bilingual_str JoinMessages(const std::vector& errors, const std::vector& warnings) +{ + bilingual_str result; + for (const auto& messages : {errors, warnings}) { + for (const auto& message : messages) { + if (!result.empty()) result += Untranslated(" "); + result += message; + } + } + return result; +} + +void MoveMessages(std::vector& src, std::vector& dest) +{ + dest.insert(dest.end(), std::make_move_iterator(src.begin()), std::make_move_iterator(src.end())); + src.clear(); +} +} // namespace detail +} // namespace util diff --git a/src/util/result.h b/src/util/result.h index 9f0a77aeeb226..e8bba4a1cb241 100644 --- a/src/util/result.h +++ b/src/util/result.h @@ -18,11 +18,11 @@ namespace util { -//! The `Result` class provides a standard way for functions to return an error -//! string in addition to optional result values. +//! The `Result` class provides a standard way for functions to return error and +//! warning strings in addition to optional result values. //! //! The easiest way to understand `Result` is to think of it as a substitute -//! for `std::optional`, that just contains an error string in +//! for `std::optional`, that just contains error and warning strings in //! addition to the optional return value. For example: //! //! util::Result AddNumbers(int a, int b) @@ -61,20 +61,44 @@ namespace util { template class Result; -//! Wrapper type to pass error strings to Result constructors. +//! Wrapper types to pass error and warning strings to Result constructors. struct Error { bilingual_str message; }; +struct Warning { + bilingual_str message; +}; + +//! Wrapper type to move error and warning strings from an existing Result object to a new Result constructor. +template +struct MoveMessages { + MoveMessages(Result&& result) : m_result(result) {} + Result& m_result; +}; + +//! Template deduction guide for MoveMessages class. +template +MoveMessages(Result&& result) -> MoveMessages; namespace detail { +//! Empty string list +const std::vector EMPTY_LIST{}; + +//! Helper function to join messages in space separated string. +bilingual_str JoinMessages(const std::vector& errors, const std::vector& warnings); + +//! Helper function to move messages from one vector to another. +void MoveMessages(std::vector& src, std::vector& dest); + //! Substitute for std::monostate that doesn't depend on std::variant. struct MonoState{}; -//! Error information only allocated on failure. +//! Error information only allocated if there are errors or warnings. template struct ErrorInfo { - std::conditional_t, MonoState, F> failure; - bilingual_str error; + std::optional, MonoState, F>> failure{}; + std::vector errors{}; + std::vector warnings{}; }; //! Result base class which is inherited by Result. @@ -91,6 +115,12 @@ class ResultBase protected: std::unique_ptr> m_info; + ErrorInfo& Info() LIFETIMEBOUND + { + if (!m_info) m_info = std::make_unique>(); + return *m_info; + } + //! Value setter methods which do nothing because this ResultBase class is //! specialized for type T=void, so there is no result value for it to hold. //! The other ResultBase specialization below for non-void T overrides these @@ -106,7 +136,7 @@ class ResultBase static void ConstructResult(Result& result, Args&&... args) { if constexpr (Failure) { - result.m_info.reset(new detail::ErrorInfo{.failure{std::forward(args)...}, .error{}}); + result.Info().failure.emplace(std::forward(args)...); } else { result.ConstructValue(std::forward(args)...); } @@ -116,12 +146,28 @@ class ResultBase template static void ConstructResult(Result& result, util::Error error, Args&&... args) { + result.AddError(std::move(error.message)); ConstructResult(result, std::forward(args)...); - result.m_info->error = std::move(error.message); } - //! Helper function to move success or failure values and any error - //! message from one result object to another. The two result + //! ConstructResult() overload peeling off a util::Warning constructor argument. + template + static void ConstructResult(Result& result, util::Warning warning, Args&&... args) + { + result.AddWarning(std::move(warning.message)); + ConstructResult(result, std::forward(args)...); + } + + //! ConstructResult() overload peeling off a util::MoveMessages constructor argument. + template + static void ConstructResult(Result& result, util::MoveMessages messages, Args&&... args) + { + result.MoveMessages(std::move(messages.m_result)); + ConstructResult(result, std::forward(args)...); + } + + //! Helper function to move success or failure values and any error and + //! warning messages from one result object to another. The two result //! objects must have compatible success and failure types. template static void MoveResult(DstResult& dst, SrcResult&& src) @@ -130,20 +176,21 @@ class ResultBase if (dst) { dst.DestroyValue(); } else { - dst.m_info.reset(); + dst.m_info->failure.reset(); } } + dst.MoveMessages(src); if (src) { dst.MoveValue(std::move(src)); } else { - dst.m_info.reset(new detail::ErrorInfo{std::move(*src.m_info)}); + dst.Info().failure = std::move(src.m_info->failure); } } - //! Disallow operator= and require explicit Result::Set() calls 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 messages. + //! Disallow potentially dangerous assignment operators which might erase + //! error and warning messages. The Result::Set() method can be used instead + //! of operator= to assign result values while keeping any existing errors + //! and warnings. template ResultBase& operator=(Result&&) = delete; @@ -152,10 +199,35 @@ class ResultBase public: //! Success check. - explicit operator bool() const { return !m_info; } + explicit operator bool() const { return !m_info || !m_info->failure; } //! Error retrieval. - const auto& GetFailure() const LIFETIMEBOUND { assert(!*this); return m_info->failure; } + const auto& GetFailure() const LIFETIMEBOUND { assert(!*this); return *m_info->failure; } + const std::vector& GetErrors() const LIFETIMEBOUND { return m_info ? m_info->errors : EMPTY_LIST; } + const std::vector& GetWarnings() const LIFETIMEBOUND { return m_info ? m_info->warnings : EMPTY_LIST; } + + //! Add error and warning messages. + void AddError(bilingual_str error) + { + if (!error.empty()) this->Info().errors.emplace_back(std::move(error)); + } + void AddWarning(bilingual_str warning) + { + if (!warning.empty()) this->Info().warnings.emplace_back(std::move(warning)); + } + + //! Move error and warning messages from another result to this one. + template + void MoveMessages(Result&& other) + { + if (other.m_info) { + // Check that errors and warnings are empty before calling + // MoveMessages to avoid allocating memory in this->Info() in the + // typical case when there are no errors or warnings. + if (!other.m_info->errors.empty()) detail::MoveMessages(other.m_info->errors, this->Info().errors); + if (!other.m_info->warnings.empty()) detail::MoveMessages(other.m_info->warnings, this->Info().warnings); + } + } }; //! Result base class for T value type. Holds value and provides accessor methods. @@ -210,9 +282,10 @@ class Result : public detail::ResultBase using Base = detail::ResultBase; public: - //! Construct a Result object setting a success or failure value. An - //! optional util::Error argument is processed first to set an error - //! message. Then, any remaining arguments are passed to the type `T` + //! Construct a Result object setting a success or failure value and + //! optional warning and error messages. Initial util::Error, util::Warning, + //! and util::MoveMessages arguments are processed first to add warning and + //! error messages. Then, any remaining arguments are passed to the type `T` //! constructor and used to construct a success value in the success case. //! In the failure case, any remaining arguments are passed to the type `F` //! constructor and used to construct a failure value. @@ -223,29 +296,53 @@ class Result : public detail::ResultBase } //! Move-construct a Result object from another Result object, moving the - //! success or failure value and any error message. + //! success or failure value any any error or warning messages. template Result(Result&& other) { Base::template MoveResult(*this, std::move(other)); } - //! Move success or failure values and any error message from another Result - //! object to this object. + //! Move success or failure values from another Result object to this + //! object. Also move any error and warning messages from the other Result + //! object to this one. If this Result object has an existing success or + //! failure value, it is cleared and replaced by the other value. If this + //! Result object has any error or warning messages, they are kept and + //! appended to. Result& Set(Result&& other) LIFETIMEBOUND { Base::template MoveResult(*this, std::move(other)); return *this; } - inline friend bilingual_str _ErrorString(const Result& result) + //! Operator moving warning and error messages from a source result object + //! to a destination result object. Only moves message strings, does not + //! change success or failure values of either Result object. + //! + //! This is useful for combining error and warning messages from multiple + //! result objects into a single object, e.g.: + //! + //! util::Result result; + //! auto r1 = DoSomething() >> result; + //! auto r2 = DoSomethingElse() >> result; + //! ... + //! return result; + //! + template + Result&& operator>>(O&& other LIFETIMEBOUND) && { - return result ? bilingual_str{} : result.m_info->error; + other.MoveMessages(*this); + return std::move(*this); } }; +//! 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 +//! are present. More complicated applications should use GetErrors() and +//! GetWarning() methods directly. template -bilingual_str ErrorString(const Result& result) { return _ErrorString(result); } +bilingual_str ErrorString(const Result& result) { return detail::JoinMessages(result.GetErrors(), result.GetWarnings()); } } // namespace util #endif // BITCOIN_UTIL_RESULT_H