diff --git a/src/Makefile.am b/src/Makefile.am index 7673d2f54511c..8e48a5a65ee76 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -750,6 +750,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 \ @@ -988,6 +989,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 bc36a6733106e..5f3ff7876840c 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::MoveFrom(result), util::Error{Untranslated("str error")}, result.GetFailure()}; + return {util::MoveFrom(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::MoveFrom(result), ret}; +} + +util::Result ChainedFailFn(FnError arg, int ret) +{ + return {util::Error{Untranslated("chained fail.")}, util::MoveFrom(EnumFailFn(arg)), util::MoveFrom(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.Update(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_update) result.Update({util::Error{Untranslated("error")}, ERR1}); ExpectFail(result, Untranslated("error"), ERR1); result.Update(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_update) result2.Update({util::Error{Untranslated("error")}, 3}); ExpectFail(result2, Untranslated("error"), 3); result2.Update(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(Assert(result.GetMessages())->errors.size(), 1); + BOOST_CHECK_EQUAL(Assert(result.GetMessages())->errors[0], Untranslated("Error.")); + BOOST_CHECK_EQUAL(Assert(result.GetMessages())->warnings.size(), 1); + BOOST_CHECK_EQUAL(Assert(result.GetMessages())->warnings[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..f847c3c637f08 --- /dev/null +++ b/src/util/result.cpp @@ -0,0 +1,33 @@ +// Copyright (c) 2024 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 Messages& messages) +{ + bilingual_str result; + for (const auto& messages : {messages.errors, messages.warnings}) { + for (const auto& message : messages) { + if (!result.empty()) result += Untranslated(" "); + result += message; + } + } + return result; +} +} // namespace detail + +void ResultTraits::MergeInto(Messages& dst, Messages& src) { + dst.errors.insert(dst.errors.end(), std::make_move_iterator(src.errors.begin()), std::make_move_iterator(src.errors.end())); + dst.warnings.insert(dst.warnings.end(), std::make_move_iterator(src.warnings.begin()), std::make_move_iterator(src.warnings.end())); + src.errors.clear(); + src.warnings.clear(); +} +} // namespace util diff --git a/src/util/result.h b/src/util/result.h index 34c8075c1ad05..47694fa6fe7a3 100644 --- a/src/util/result.h +++ b/src/util/result.h @@ -17,9 +17,15 @@ #include namespace util { +//! Default MessagesType, simple list of errors and warnings. +struct Messages { + std::vector errors{}; + std::vector warnings{}; +}; + //! The Result class provides an //! efficient way for functions to return structured result information, as well -//! as descriptive error messages. +//! as descriptive error and warning messages. //! //! Logically, a result object is equivalent to: //! @@ -58,15 +64,78 @@ namespace util { //! return function results. //! //! Usage examples can be found in \example ../test/result_tests.cpp. -template +template class Result; //! Wrapper object to pass an error string to the Result constructor. struct Error { bilingual_str message; }; +//! Wrapper object to pass a warning string to the Result constructor. +struct Warning { + bilingual_str message; +}; + +//! Wrapper object to pass an existing Result object to the Result constructor, +//! moving messages into the new object, like operator>> below. Passing +//! MoveFrom(result) to the Result constructor is similar to passing +//! std::move(result), except it doesn't require the two result objects to have +//! compatible Success and Failure types, and it only moves message and info +//! fields, leaving success and value fields alone. Like operator>>, it is +//! useful for combining data from many result objects into a single result. +template +struct MoveFrom { + MoveFrom(Result&& result) : m_result(result) {} + Result& m_result; +}; + +//! Template deduction guide for MoveFrom class. +template +MoveFrom(Result&& result) -> MoveFrom; + +//! Type trait that can be specialized to control the way SuccessType / +//! FailureType / MessagesType values are combined. +template +struct ResultTraits { + template + static void MergeInto(O& dst, T& src) + { + dst = std::move(src); + } +}; + +//! Type trait that can be specialized to let the Result class work with custom +//! MessagesType types holding error and warning messages. +template +struct MessagesTraits; + +//! ResultTraits specialization for Messages struct. +template<> +struct ResultTraits { + static void MergeInto(Messages& dst, Messages& src); +}; + +//! MessagesTraits specialization for Messages struct. +template<> +struct MessagesTraits { + static void AddError(Messages& messages, bilingual_str error) + { + messages.errors.emplace_back(std::move(error)); + } + static void AddWarning(Messages& messages, bilingual_str warning) + { + messages.warnings.emplace_back(std::move(warning)); + } + static bool HasMessages(const Messages& messages) + { + return messages.errors.size() || messages.warnings.size(); + } +}; namespace detail { +//! Helper function to join messages in space separated string. +bilingual_str JoinMessages(const Messages& messages); + //! Substitute for std::monostate that doesn't depend on std::variant. struct Monostate{}; @@ -157,9 +226,10 @@ class Result : public detail::SuccessHolder requires (std::decay_t::is_result) Result(O&& other) @@ -178,7 +248,10 @@ class Result : public detail::SuccessHolder(*this, other); } - //! Update this result by moving from another result object. + //! Update this result by moving from another result object. Existing + //! success, failure, and messages values are merged (using + //! ResultTraits::MergeInto specializations), so errors and warning messages + //! get appended instead of overwriting existing ones. Result& Update(Result&& other) LIFETIMEBOUND { Move(*this, other); @@ -190,12 +263,19 @@ class Result : public detail::SuccessHolder Result& operator=(Result&&) = delete; + void AddError(bilingual_str error) + { + if (!error.empty()) MessagesTraits::AddError(this->EnsureFailData().messages, std::move(error)); + } + void AddWarning(bilingual_str warning) + { + if (!warning.empty()) MessagesTraits::AddWarning(this->EnsureFailData().messages, std::move(warning)); + } + protected: template friend class Result; @@ -220,12 +300,31 @@ class Result : public detail::SuccessHolder static void Construct(Result& result, util::Error error, Args&&... args) { - result.EnsureFailData().messages = std::move(error.message); + result.AddError(std::move(error.message)); Construct(result, std::forward(args)...); } + //! Construct() overload peeling off a util::Warning constructor argument. + template + static void Construct(Result& result, util::Warning warning, Args&&... args) + { + result.AddWarning(std::move(warning.message)); + Construct(result, std::forward(args)...); + } + + //! Construct() overload peeling off a util::MoveFrom constructor argument. + template + static void Construct(Result& result, MoveFrom other, Args&&... args) + { + other.m_result >> result; + Construct(result, std::forward(args)...); + } + //! Move success, failure, and messages from source Result object to - //! destination object. The source and destination results are not required + //! destination object. Existing values are merged (using + //! ResultTraits::MergeInto specializations), so destination errors and + //! warning messages get appended to instead of overwritten. + //! The source and destination results are not required //! to have the same types, and assigning void source types to non-void //! destinations type is allowed, since no source information is lost. But //! assigning non-void source types to void destination types is not @@ -233,16 +332,27 @@ class Result : public detail::SuccessHolder static void Move(DstResult& dst, SrcResult& src) { - // Move messages values first, then move success or failure value below. - if (src.GetMessages() && !src.GetMessages()->empty()) { - dst.EnsureMessages() = std::move(*src.GetMessages()); - } + // Use operator>> to move messages values first, then move success or + // failure value below. + src >> dst; // If DstConstructed is true, it means dst has either a success value or - // a failure value set, which needs to be destroyed and replaced. If + // a failure value set, which needs to be merged or replaced. If // DstConstructed is false, then dst is being constructed now and has no // values set. if constexpr (DstConstructed) { - if (dst) { + if (dst && src) { + // dst and src both hold success values, so merge them and return + if constexpr (!std::is_same_v) { + ResultTraits::MergeInto(*dst, *src); + } + return; + } else if (!dst && !src) { + // dst and src both hold failure values, so merge them and return + if constexpr (!std::is_same_v) { + ResultTraits::MergeInto(dst.GetFailure(), src.GetFailure()); + } + return; + } else if (dst) { // dst has a success value, so destroy it before replacing it with src failure value if constexpr (!std::is_same_v) { using DstSuccess = typename DstResult::SuccessType; @@ -280,10 +390,40 @@ class Result : public detail::SuccessHolder result; +//! auto r1 = DoSomething() >> result; +//! auto r2 = DoSomethingElse() >> result; +//! ... +//! return result; +//! +template +requires (std::decay_t::is_result) +decltype(auto) operator>>(SrcResult&& src LIFETIMEBOUND, DstResult&& dst) +{ + using SrcType = std::decay_t; + if (src.GetMessages() && MessagesTraits::HasMessages(*src.GetMessages())) { + ResultTraits::MergeInto(dst.EnsureMessages(), *src.GetMessages()); + } + return static_cast(src); +} + +//! 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 !result && result.GetMessages() ? *result.GetMessages() : bilingual_str{}; + const auto* messages{result.GetMessages()}; + return messages ? detail::JoinMessages(*messages) : bilingual_str{}; } } // namespace util