From bcbfb049922a5d8eb4b06f99c87a3172e4cdf9d6 Mon Sep 17 00:00:00 2001
From: Mayukha Vadari <mvadari@gmail.com>
Date: Mon, 16 Dec 2024 15:00:14 -0800
Subject: [PATCH] fix: allow overlapping types in `Expected` (#5218)

For example, Expected<std::uint32_t, Json::Value>, will now build even though there is animplicit conversion from unsigned int to Json::Value.
---
 include/xrpl/basics/Expected.h    |  6 ++++--
 src/test/basics/Expected_test.cpp | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/include/xrpl/basics/Expected.h b/include/xrpl/basics/Expected.h
index 10f188af11..ced41b13ca 100644
--- a/include/xrpl/basics/Expected.h
+++ b/include/xrpl/basics/Expected.h
@@ -137,13 +137,15 @@ class [[nodiscard]] Expected
 public:
     template <typename U>
         requires std::convertible_to<U, T>
-    constexpr Expected(U&& r) : Base(T(std::forward<U>(r)))
+    constexpr Expected(U&& r)
+        : Base(boost::outcome_v2::in_place_type_t<T>{}, std::forward<U>(r))
     {
     }
 
     template <typename U>
         requires std::convertible_to<U, E> && (!std::is_reference_v<U>)
-    constexpr Expected(Unexpected<U> e) : Base(E(std::move(e.value())))
+    constexpr Expected(Unexpected<U> e)
+        : Base(boost::outcome_v2::in_place_type_t<E>{}, std::move(e.value()))
     {
     }
 
diff --git a/src/test/basics/Expected_test.cpp b/src/test/basics/Expected_test.cpp
index d60809aee1..2f8b92eb91 100644
--- a/src/test/basics/Expected_test.cpp
+++ b/src/test/basics/Expected_test.cpp
@@ -84,6 +84,29 @@ struct Expected_test : beast::unit_test::suite
             }
             BEAST_EXPECT(throwOccurred);
         }
+        // Test non-error overlapping type construction.
+        {
+            auto expected = []() -> Expected<std::uint32_t, std::uint16_t> {
+                return 1;
+            }();
+            BEAST_EXPECT(expected);
+            BEAST_EXPECT(expected.has_value());
+            BEAST_EXPECT(expected.value() == 1);
+            BEAST_EXPECT(*expected == 1);
+
+            bool throwOccurred = false;
+            try
+            {
+                // There's no error, so should throw.
+                [[maybe_unused]] std::uint16_t const t = expected.error();
+            }
+            catch (std::runtime_error const& e)
+            {
+                BEAST_EXPECT(e.what() == std::string("bad expected access"));
+                throwOccurred = true;
+            }
+            BEAST_EXPECT(throwOccurred);
+        }
         // Test error construction from rvalue.
         {
             auto const expected = []() -> Expected<std::string, TER> {