refactor: Drop util::Result operator=

`util::Result` objects are aggregates that can hold multiple fields with
different information. Currently Result objects can only hold a success value
of an arbitrary type or a single bilingual_str error message. In followup PR
https://github.com/bitcoin/bitcoin/pull/25722, Result objects may be able to
hold both success and failure values of different types, plus error and warning
messages.

Having a Result::operator= assignment operator that completely erases all
existing Result information before assigning new information is potentially
dangerous in this case. For example, code that looks like it is assigning a
warning value could erase previously-assigned success or failure values.
Conversely, code that looks like it is just assigning a success or failure
value could erase previously assigned error and warning messages.

To prevent potential bugs like this, disable Result::operator= assignment
operator.

It is possible in the future we may want to re-enable operator= in limited
cases (such as when implicit conversions are not used) or add a Replace() or
Reset() method that mimicks default operator= behavior. Followup PR
https://github.com/bitcoin/bitcoin/pull/25722 also adds a Result::Update()
method providing another way to update an existing Result object.

Co-authored-by: stickies-v <stickies-v@protonmail.com>

1 files changed, 10 insertions, 0 deletions

diff --git a/src/util/result.h b/src/util/result.h
index b99995c7e5..11fe89af25 100644
--- a/src/util/result.h
+++ b/src/util/result.h
@@ -39,6 +39,13 @@ private:
 
     std::variant<bilingual_str, T> m_variant;
 
+    //! 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
+    //! messages.
+    Result& operator=(const Result&) = delete;
+    Result& operator=(Result&&) = delete;
+
     template <typename FT>
     friend bilingual_str ErrorString(const Result<FT>& result);
 
@@ -46,6 +53,9 @@ public:
     Result() : m_variant{std::in_place_index_t<1>{}, std::monostate{}} {}  // constructor for void
     Result(T obj) : m_variant{std::in_place_index_t<1>{}, std::move(obj)} {}
     Result(Error error) : m_variant{std::in_place_index_t<0>{}, std::move(error.message)} {}
+    Result(const Result&) = default;
+    Result(Result&&) = default;
+    ~Result() = default;
 
     //! std::optional methods, so functions returning optional<T> can change to
     //! return Result<T> with minimal changes to existing code, and vice versa.