[llvm] [orc-rt] Clean up SPS serialization for Error, Expected; add testcase. (PR #157029)

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 5 00:12:52 PDT 2025


https://github.com/lhames created https://github.com/llvm/llvm-project/pull/157029

This commit cleans up the SPS serialization code for Error and Expected, and adds test cases for success and failure values of each.

>From 93271e96a1cf2c8716a32001963d75c330fdea64 Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Fri, 5 Sep 2025 16:11:29 +1000
Subject: [PATCH] [orc-rt] Clean up SPS serialization for Error, Expected; add
 testcase.

This commit cleans up the SPS serialization code for Error and Expected, and
adds test cases for success and failure values of each.
---
 .../orc-rt/SimplePackedSerialization.h        | 214 +++++++++---------
 .../SimplePackedSerializationTest.cpp         |  75 ++++++
 .../SimplePackedSerializationTestUtils.h      |  21 +-
 3 files changed, 199 insertions(+), 111 deletions(-)

diff --git a/orc-rt/include/orc-rt/SimplePackedSerialization.h b/orc-rt/include/orc-rt/SimplePackedSerialization.h
index b01e017d1529b..95e10bc437cfb 100644
--- a/orc-rt/include/orc-rt/SimplePackedSerialization.h
+++ b/orc-rt/include/orc-rt/SimplePackedSerialization.h
@@ -513,12 +513,6 @@ template <> class SPSSerializationTraits<SPSString, std::string_view> {
 /// SPS tag type for errors.
 class SPSError;
 
-/// SPS tag type for expecteds, which are either a T or a string representing
-/// an error.
-template <typename SPSTagT> class SPSExpected;
-
-namespace detail {
-
 /// Helper type for serializing Errors.
 ///
 /// llvm::Errors are move-only, and not inspectable except by consuming them.
@@ -529,139 +523,141 @@ namespace detail {
 /// The SPSSerializableError type is a helper that can be
 /// constructed from an llvm::Error, but inspected more than once.
 struct SPSSerializableError {
-  bool HasError = false;
-  std::string ErrMsg;
-};
-
-/// Helper type for serializing Expected<T>s.
-///
-/// See SPSSerializableError for more details.
-///
-// FIXME: Use std::variant for storage once we have c++17.
-template <typename T> struct SPSSerializableExpected {
-  bool HasValue = false;
-  T Value{};
-  std::string ErrMsg;
-};
-
-inline SPSSerializableError toSPSSerializable(Error Err) {
-  if (Err)
-    return {true, toString(std::move(Err))};
-  return {false, {}};
-}
-
-inline Error fromSPSSerializable(SPSSerializableError BSE) {
-  if (BSE.HasError)
-    return make_error<StringError>(BSE.ErrMsg);
-  return Error::success();
-}
-
-template <typename T>
-SPSSerializableExpected<T> toSPSSerializable(Expected<T> E) {
-  if (E)
-    return {true, std::move(*E), {}};
-  else
-    return {false, {}, toString(E.takeError())};
-}
+  SPSSerializableError() = default;
+  SPSSerializableError(Error Err) {
+    if (Err)
+      Msg = toString(std::move(Err));
+  }
 
-template <typename T>
-Expected<T> fromSPSSerializable(SPSSerializableExpected<T> BSE) {
-  if (BSE.HasValue)
-    return std::move(BSE.Value);
-  else
-    return make_error<StringError>(BSE.ErrMsg);
-}
+  Error toError() {
+    if (Msg)
+      return make_error<StringError>(std::move(*Msg));
+    return Error::success();
+  }
 
-} // namespace detail
+  std::optional<std::string> Msg;
+};
 
-/// Serialize to a SPSError from a detail::SPSSerializableError.
-template <>
-class SPSSerializationTraits<SPSError, detail::SPSSerializableError> {
+template <> class SPSSerializationTraits<SPSError, SPSSerializableError> {
 public:
-  static size_t size(const detail::SPSSerializableError &BSE) {
-    size_t Size = SPSArgList<bool>::size(BSE.HasError);
-    if (BSE.HasError)
-      Size += SPSArgList<SPSString>::size(BSE.ErrMsg);
-    return Size;
+  static size_t size(const SPSSerializableError &E) {
+    if (E.Msg)
+      return SPSArgList<bool, SPSString>::size(true, *E.Msg);
+    else
+      return SPSArgList<bool>::size(false);
   }
 
-  static bool serialize(SPSOutputBuffer &OB,
-                        const detail::SPSSerializableError &BSE) {
-    if (!SPSArgList<bool>::serialize(OB, BSE.HasError))
+  static bool serialize(SPSOutputBuffer &OB, const SPSSerializableError &E) {
+    if (E.Msg)
+      return SPSArgList<bool, SPSString>::serialize(OB, true, *E.Msg);
+    else
+      return SPSArgList<bool>::serialize(OB, false);
+  }
+
+  static bool deserialize(SPSInputBuffer &IB, SPSSerializableError &E) {
+    bool HasError = false;
+    if (!SPSArgList<bool>::deserialize(IB, HasError))
       return false;
-    if (BSE.HasError)
-      if (!SPSArgList<SPSString>::serialize(OB, BSE.ErrMsg))
+    if (HasError) {
+      std::string Msg;
+      if (!SPSArgList<SPSString>::deserialize(IB, Msg))
         return false;
+      E.Msg = std::move(Msg);
+    } else
+      E.Msg = std::nullopt;
     return true;
   }
+};
 
-  static bool deserialize(SPSInputBuffer &IB,
-                          detail::SPSSerializableError &BSE) {
-    if (!SPSArgList<bool>::deserialize(IB, BSE.HasError))
-      return false;
+/// SPS tag type for expecteds, which are either a T or a string representing
+/// an error.
+template <typename SPSTagT> class SPSExpected;
 
-    if (!BSE.HasError)
-      return true;
+/// Helper type for serializing Expected<T>s.
+///
+/// See SPSSerializableError for more details.
+template <typename T> struct SPSSerializableExpected {
+  SPSSerializableExpected() = default;
+  SPSSerializableExpected(Expected<T> E) {
+    if (E)
+      Val = decltype(Val)(std::in_place_index<0>, std::move(*E));
+    else
+      Val = decltype(Val)(std::in_place_index<1>, toString(E.takeError()));
+  }
+  SPSSerializableExpected(Error E) {
+    assert(E && "Cannot create Expected from Error::success()");
+    Val = decltype(Val)(std::in_place_index<1>, toString(std::move(E)));
+  }
 
-    return SPSArgList<SPSString>::deserialize(IB, BSE.ErrMsg);
+  Expected<T> toExpected() {
+    if (Val.index() == 0)
+      return Expected<T>(std::move(std::get<0>(Val)));
+    return Expected<T>(make_error<StringError>(std::move(std::get<1>(Val))));
   }
+
+  std::variant<T, std::string> Val{std::in_place_index<0>, T()};
 };
 
-/// Serialize to a SPSExpected<SPSTagT> from a
-/// detail::SPSSerializableExpected<T>.
+template <typename T>
+SPSSerializableExpected<T> toSPSSerializableExpected(Expected<T> E) {
+  return std::move(E);
+}
+
+template <typename T>
+SPSSerializableExpected<T> toSPSSerializableExpected(Error E) {
+  return std::move(E);
+}
+
 template <typename SPSTagT, typename T>
-class SPSSerializationTraits<SPSExpected<SPSTagT>,
-                             detail::SPSSerializableExpected<T>> {
+class SPSSerializationTraits<SPSExpected<SPSTagT>, SPSSerializableExpected<T>> {
 public:
-  static size_t size(const detail::SPSSerializableExpected<T> &BSE) {
-    size_t Size = SPSArgList<bool>::size(BSE.HasValue);
-    if (BSE.HasValue)
-      Size += SPSArgList<SPSTagT>::size(BSE.Value);
+  static size_t size(const SPSSerializableExpected<T> &E) {
+    if (E.Val.index() == 0)
+      return SPSArgList<bool, SPSTagT>::size(true, std::get<0>(E.Val));
     else
-      Size += SPSArgList<SPSString>::size(BSE.ErrMsg);
-    return Size;
+      return SPSArgList<bool, SPSString>::size(false, std::get<1>(E.Val));
   }
 
   static bool serialize(SPSOutputBuffer &OB,
-                        const detail::SPSSerializableExpected<T> &BSE) {
-    if (!SPSArgList<bool>::serialize(OB, BSE.HasValue))
-      return false;
-
-    if (BSE.HasValue)
-      return SPSArgList<SPSTagT>::serialize(OB, BSE.Value);
-
-    return SPSArgList<SPSString>::serialize(OB, BSE.ErrMsg);
+                        const SPSSerializableExpected<T> &E) {
+    if (E.Val.index() == 0)
+      return SPSArgList<bool, SPSTagT>::serialize(OB, true, std::get<0>(E.Val));
+    else
+      return SPSArgList<bool, SPSString>::serialize(OB, false,
+                                                    std::get<1>(E.Val));
   }
 
-  static bool deserialize(SPSInputBuffer &IB,
-                          detail::SPSSerializableExpected<T> &BSE) {
-    if (!SPSArgList<bool>::deserialize(IB, BSE.HasValue))
+  static bool deserialize(SPSInputBuffer &IB, SPSSerializableExpected<T> &E) {
+    bool HasValue = false;
+    if (!SPSArgList<bool>::deserialize(IB, HasValue))
       return false;
-
-    if (BSE.HasValue)
-      return SPSArgList<SPSTagT>::deserialize(IB, BSE.Value);
-
-    return SPSArgList<SPSString>::deserialize(IB, BSE.ErrMsg);
+    if (HasValue) {
+      T Val;
+      if (!SPSArgList<SPSTagT>::deserialize(IB, Val))
+        return false;
+      E.Val = decltype(E.Val){std::in_place_index<0>, std::move(Val)};
+    } else {
+      std::string Msg;
+      if (!SPSArgList<SPSString>::deserialize(IB, Msg))
+        return false;
+      E.Val = decltype(E.Val){std::in_place_index<1>, std::move(Msg)};
+    }
+    return true;
   }
 };
 
-/// Serialize to a SPSExpected<SPSTagT> from a detail::SPSSerializableError.
+/// Serialize to a SPSExpected<SPSTagT> from a SPSSerializableError.
 template <typename SPSTagT>
-class SPSSerializationTraits<SPSExpected<SPSTagT>,
-                             detail::SPSSerializableError> {
+class SPSSerializationTraits<SPSExpected<SPSTagT>, SPSSerializableError> {
 public:
-  static size_t size(const detail::SPSSerializableError &BSE) {
-    assert(BSE.HasError && "Cannot serialize expected from a success value");
-    return SPSArgList<bool>::size(false) +
-           SPSArgList<SPSString>::size(BSE.ErrMsg);
+  static size_t size(const SPSSerializableError &SE) {
+    assert(SE.Msg && "Cannot serialize expected from a success value");
+    return SPSArgList<bool, SPSString>::size(false, *SE.Msg);
   }
 
-  static bool serialize(SPSOutputBuffer &OB,
-                        const detail::SPSSerializableError &BSE) {
-    assert(BSE.HasError && "Cannot serialize expected from a success value");
-    if (!SPSArgList<bool>::serialize(OB, false))
-      return false;
-    return SPSArgList<SPSString>::serialize(OB, BSE.ErrMsg);
+  static bool serialize(SPSOutputBuffer &OB, const SPSSerializableError &SE) {
+    assert(SE.Msg && "Cannot serialize expected from a success value");
+    return SPSArgList<bool, SPSString>::serialize(OB, false, *SE.Msg);
   }
 };
 
@@ -674,9 +670,7 @@ class SPSSerializationTraits<SPSExpected<SPSTagT>, T> {
   }
 
   static bool serialize(SPSOutputBuffer &OB, const T &Value) {
-    if (!SPSArgList<bool>::serialize(OB, true))
-      return false;
-    return SPSArgList<SPSTagT>::serialize(Value);
+    return SPSArgList<bool, SPSTagT>::serialize(OB, true, Value);
   }
 };
 
diff --git a/orc-rt/unittests/SimplePackedSerializationTest.cpp b/orc-rt/unittests/SimplePackedSerializationTest.cpp
index 6c58503f0797a..9ccedef69628f 100644
--- a/orc-rt/unittests/SimplePackedSerializationTest.cpp
+++ b/orc-rt/unittests/SimplePackedSerializationTest.cpp
@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "orc-rt/SimplePackedSerialization.h"
+
 #include "SimplePackedSerializationTestUtils.h"
 #include "gtest/gtest.h"
 
@@ -182,3 +183,77 @@ TEST(SimplePackedSerializationTest, ArgListSerialization) {
   EXPECT_EQ(Arg2, ArgOut2);
   EXPECT_EQ(Arg3, ArgOut3);
 }
+
+TEST(SimplePackedSerializationTest, SerializeErrorSuccess) {
+  auto B = spsSerialize<SPSArgList<SPSError>>(
+      SPSSerializableError(Error::success()));
+  if (!B) {
+    ADD_FAILURE() << "Unexpected failure to serialize error-success value";
+    return;
+  }
+  SPSSerializableError SE;
+  if (!spsDeserialize<SPSArgList<SPSError>>(*B, SE)) {
+    ADD_FAILURE() << "Unexpected failure to deserialize error-success value";
+    return;
+  }
+
+  auto E = SE.toError();
+  EXPECT_FALSE(!!E); // Expect non-error, i.e. Error::success().
+}
+
+TEST(SimplePackedSerializationTest, SerializeErrorFailure) {
+  auto B = spsSerialize<SPSArgList<SPSError>>(
+      SPSSerializableError(make_error<StringError>("test error message")));
+  if (!B) {
+    ADD_FAILURE() << "Unexpected failure to serialize error-failure value";
+    return;
+  }
+  SPSSerializableError SE;
+  if (!spsDeserialize<SPSArgList<SPSError>>(*B, SE)) {
+    ADD_FAILURE() << "Unexpected failure to deserialize error-failure value";
+    return;
+  }
+
+  EXPECT_EQ(toString(SE.toError()), std::string("test error message"));
+}
+
+TEST(SimplePackedSerializationTest, SerializeExpectedSuccess) {
+  auto B = spsSerialize<SPSArgList<SPSExpected<uint32_t>>>(
+      toSPSSerializableExpected(Expected<uint32_t>(42U)));
+  if (!B) {
+    ADD_FAILURE() << "Unexpected failure to serialize expected-success value";
+    return;
+  }
+  SPSSerializableExpected<uint32_t> SE;
+  if (!spsDeserialize<SPSArgList<SPSExpected<uint32_t>>>(*B, SE)) {
+    ADD_FAILURE() << "Unexpected failure to deserialize expected-success value";
+    return;
+  }
+
+  auto E = SE.toExpected();
+  if (E)
+    EXPECT_EQ(*E, 42U);
+  else
+    ADD_FAILURE() << "Unexpected failure value";
+}
+
+TEST(SimplePackedSerializationTest, SerializeExpectedFailure) {
+  auto B = spsSerialize<SPSArgList<SPSExpected<uint32_t>>>(
+      toSPSSerializableExpected<uint32_t>(
+          make_error<StringError>("test error message")));
+  if (!B) {
+    ADD_FAILURE() << "Unexpected failure to serialize expected-failure value";
+    return;
+  }
+  SPSSerializableExpected<uint32_t> SE;
+  if (!spsDeserialize<SPSArgList<SPSExpected<uint32_t>>>(*B, SE)) {
+    ADD_FAILURE() << "Unexpected failure to deserialize expected-failure value";
+    return;
+  }
+
+  auto E = SE.toExpected();
+  if (E)
+    ADD_FAILURE() << "Unexpected failure value";
+  else
+    EXPECT_EQ(toString(E.takeError()), std::string("test error message"));
+}
diff --git a/orc-rt/unittests/SimplePackedSerializationTestUtils.h b/orc-rt/unittests/SimplePackedSerializationTestUtils.h
index 5468045f5fbe7..7bfa37b6d4bda 100644
--- a/orc-rt/unittests/SimplePackedSerializationTestUtils.h
+++ b/orc-rt/unittests/SimplePackedSerializationTestUtils.h
@@ -10,10 +10,29 @@
 #define ORC_RT_UNITTEST_SIMPLEPACKEDSERIALIZATIONTESTUTILS_H
 
 #include "orc-rt/SimplePackedSerialization.h"
+#include "orc-rt/WrapperFunction.h"
 #include "gtest/gtest.h"
 
+#include <optional>
+
+template <typename SPSTraitsT, typename... ArgTs>
+static inline std::optional<orc_rt::WrapperFunctionBuffer>
+spsSerialize(const ArgTs &...Args) {
+  auto B = orc_rt::WrapperFunctionBuffer::allocate(SPSTraitsT::size(Args...));
+  orc_rt::SPSOutputBuffer OB(B.data(), B.size());
+  if (!SPSTraitsT::serialize(OB, Args...))
+    return std::nullopt;
+  return B;
+}
+
+template <typename SPSTraitsT, typename... ArgTs>
+static bool spsDeserialize(orc_rt::WrapperFunctionBuffer &B, ArgTs &...Args) {
+  orc_rt::SPSInputBuffer IB(B.data(), B.size());
+  return SPSTraitsT::deserialize(IB, Args...);
+}
+
 template <typename SPSTagT, typename T>
-static void blobSerializationRoundTrip(const T &Value) {
+static inline void blobSerializationRoundTrip(const T &Value) {
   using BST = orc_rt::SPSSerializationTraits<SPSTagT, T>;
 
   size_t Size = BST::size(Value);



More information about the llvm-commits mailing list