Merge pull request #5288 from tchaikov/serde-more-concepts

serde: use concepts instead of type traits implemented using SFINAE

src/v/serde/envelope.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include <cinttypes>
+#include <concepts>
 #include <type_traits>
 
 namespace serde {
@@ -67,77 +68,17 @@ struct checksum_envelope {
     static constexpr auto redpanda_serde_build_checksum = true;
 };
 
-namespace detail {
-
-template<typename T, typename = void>
-struct has_compat_attribute : std::false_type {};
-
-template<typename T>
-struct has_compat_attribute<
-  T,
-  std::void_t<decltype(std::declval<T>().redpanda_serde_compat_version)>>
-  : std::true_type {};
-
-template<typename T, typename = void>
-struct has_version_attribute : std::false_type {};
-
-template<typename T>
-struct has_version_attribute<
-  T,
-  std::void_t<decltype(std::declval<T>().redpanda_serde_version)>>
-  : std::true_type {};
-
-template<typename T, typename = void>
-struct inherits_from_envelope : std::false_type {};
-
-template<typename T>
-struct inherits_from_envelope<
-  T,
-  std::void_t<decltype(std::declval<T>().redpanda_inherits_from_envelope)>>
-  : std::true_type {};
-
-template<typename T>
-struct compat_version_has_serde_version_type {
-    static constexpr auto const value = std::is_same_v<
-      std::decay_t<decltype(std::declval<T>().redpanda_serde_compat_version)>,
-      version_t>;
+template<typename T, typename Version = const serde::version_t&>
+concept is_envelope = requires {
+    { T::redpanda_serde_version } -> std::same_as<Version>;
+    { T::redpanda_serde_compat_version } -> std::same_as<Version>;
 };
 
 template<typename T>
-struct version_has_serde_version_type {
-    static constexpr auto const value = std::is_same_v<
-      std::decay_t<decltype(std::declval<T>().redpanda_serde_version)>,
-      version_t>;
-};
-
-template<typename T, typename = void>
-struct has_checksum_attribute : std::false_type {};
-
-template<typename T>
-struct has_checksum_attribute<
-  T,
-  std::void_t<decltype(std::declval<T>().redpanda_serde_build_checksum)>>
-  : std::true_type {};
-
-} // namespace detail
-
-template<typename T>
-inline constexpr auto const is_envelope_v = std::conjunction_v<
-  detail::has_compat_attribute<T>,
-  detail::has_version_attribute<T>,
-  detail::compat_version_has_serde_version_type<T>,
-  detail::version_has_serde_version_type<T>>;
-
-template<typename T>
-inline constexpr auto const is_checksum_envelope_v = std::conjunction_v<
-  detail::has_compat_attribute<T>,
-  detail::has_version_attribute<T>,
-  detail::compat_version_has_serde_version_type<T>,
-  detail::version_has_serde_version_type<T>,
-  detail::has_checksum_attribute<T>>;
+concept is_checksum_envelope
+  = is_envelope<T> && T::redpanda_serde_build_checksum;
 
 template<typename T>
-inline constexpr auto const inherits_from_envelope_v
-  = detail::inherits_from_envelope<T>::value;
+concept inherits_from_envelope = T::redpanda_inherits_from_envelope;
 
 } // namespace serde

src/v/serde/envelope_for_each_field.h

@@ -19,31 +19,21 @@ namespace serde {
 
 namespace detail {
 
-template<typename T, typename = void>
-struct has_serde_fields : std::false_type {};
-
-template<typename T>
-struct has_serde_fields<
-  T,
-  std::void_t<decltype(std::declval<std::decay_t<T>>().serde_fields())>>
-  : std::true_type {};
-
 template<typename T>
-inline constexpr auto const has_serde_fields_v = has_serde_fields<T>::value;
+concept has_serde_fields = requires(T t) {
+    t.serde_fields();
+};
 
 } // namespace detail
 
-template<
-  typename T,
-  std::enable_if_t<detail::has_serde_fields_v<T>, void*> = nullptr>
+template<detail::has_serde_fields T>
 constexpr inline auto envelope_to_tuple(T&& t) {
     return t.serde_fields();
 }
 
-template<
-  typename T,
-  std::enable_if_t<!detail::has_serde_fields_v<T>, void*> = nullptr>
-constexpr inline auto envelope_to_tuple(T& t) {
+template<typename T>
+requires(!detail::has_serde_fields<T>) constexpr inline auto envelope_to_tuple(
+  T& t) {
     static_assert(std::is_aggregate_v<T>);
     static_assert(std::is_standard_layout_v<T>);
     static_assert(!std::is_polymorphic_v<T>);
@@ -212,11 +202,14 @@ constexpr inline auto envelope_to_tuple(T& t) {
     }
 }
 
-template<typename T, typename Fn>
-inline auto envelope_for_each_field(T& t, Fn&& fn) -> std::enable_if_t<
-  !std::is_convertible_v<decltype(fn(std::declval<int&>())), bool>> {
-    static_assert(is_envelope_v<std::decay_t<T>>);
-    if constexpr (inherits_from_envelope_v<std::decay_t<T>>) {
+template<typename Fn>
+concept check_for_more_fn = requires(Fn&& fn, int& f) {
+    { fn(f) } -> std::convertible_to<bool>;
+};
+
+template<is_envelope T, typename Fn>
+inline auto envelope_for_each_field(T& t, Fn&& fn) {
+    if constexpr (inherits_from_envelope<std::decay_t<T>>) {
         std::apply(
           [&](auto&&... args) { (fn(args), ...); }, envelope_to_tuple(t));
     } else {
@@ -225,11 +218,9 @@ inline auto envelope_for_each_field(T& t, Fn&& fn) -> std::enable_if_t<
     }
 }
 
-template<typename T, typename Fn>
-inline auto envelope_for_each_field(T& t, Fn&& fn) -> std::enable_if_t<
-  std::is_convertible_v<decltype(fn(std::declval<int&>())), bool>> {
-    static_assert(is_envelope_v<std::decay_t<T>>);
-    if constexpr (inherits_from_envelope_v<std::decay_t<T>>) {
+template<is_envelope T, check_for_more_fn Fn>
+inline auto envelope_for_each_field(T& t, Fn&& fn) {
+    if constexpr (inherits_from_envelope<std::decay_t<T>>) {
         std::apply(
           [&](auto&&... args) { (void)(fn(args) && ...); },
           envelope_to_tuple(t));

src/v/serde/serde.h

@@ -25,6 +25,7 @@
 #include "utils/named_type.h"
 #include "vlog.h"
 
+#include <seastar/core/future.hh>
 #include <seastar/net/inet_address.hh>
 
 #include <absl/container/node_hash_map.h>
@@ -69,57 +70,26 @@ struct header {
     checksum_t _checksum;
 };
 
-template<typename T, typename = void>
-struct help_has_serde_read : std::false_type {};
-
-template<typename T>
-struct help_has_serde_read<
-  T,
-  std::void_t<decltype(std::declval<T>().serde_read(
-    std::declval<std::add_lvalue_reference_t<iobuf_parser>>(),
-    std::declval<header>()))>> : std::true_type {};
-
-template<typename T>
-inline constexpr auto const has_serde_read = help_has_serde_read<T>::value;
-
-template<typename T, typename = void>
-struct help_has_serde_write : std::false_type {};
-
-template<typename T>
-struct help_has_serde_write<
-  T,
-  std::void_t<decltype(std::declval<T>().serde_write(
-    std::declval<std::add_lvalue_reference_t<iobuf>>()))>> : std::true_type {};
-
 template<typename T>
-inline constexpr auto const has_serde_write = help_has_serde_write<T>::value;
-
-template<typename T, typename = void>
-struct help_has_serde_async_read : std::false_type {};
-
-template<typename T>
-struct help_has_serde_async_read<
-  T,
-  std::void_t<decltype(std::declval<T>().serde_async_read(
-    std::declval<std::add_lvalue_reference_t<iobuf_parser>>(),
-    std::declval<header>()))>> : std::true_type {};
+concept has_serde_read = requires(T t, iobuf_parser& in, const header& h) {
+    t.serde_read(in, h);
+};
 
 template<typename T>
-inline constexpr auto const has_serde_async_read
-  = help_has_serde_async_read<T>::value;
-
-template<typename T, typename = void>
-struct help_has_serde_async_write : std::false_type {};
+concept has_serde_write = requires(T t, iobuf& out) {
+    t.serde_write(out);
+};
 
 template<typename T>
-struct help_has_serde_async_write<
-  T,
-  std::void_t<decltype(std::declval<T>().serde_async_write(
-    std::declval<std::add_lvalue_reference_t<iobuf>>()))>> : std::true_type {};
+concept has_serde_async_read
+  = requires(T t, iobuf_parser& in, const header& h) {
+    { t.serde_async_read(in, h) } -> seastar::Future;
+};
 
 template<typename T>
-inline constexpr auto const has_serde_async_write
-  = help_has_serde_async_write<T>::value;
+concept has_serde_async_write = requires(T t, iobuf& out) {
+    { t.serde_async_write(out) } -> seastar::Future;
+};
 
 using serde_enum_serialized_t = int32_t;
 
@@ -159,7 +129,7 @@ inline constexpr bool is_absl_node_hash_map_v = is_absl_node_hash_map<T>::value;
 
 template<typename T>
 inline constexpr auto const is_serde_compatible_v
-  = is_envelope_v<T>
+  = is_envelope<T>
     || (std::is_scalar_v<T>  //
          && (!std::is_same_v<float, T> || std::numeric_limits<float>::is_iec559)
          && (!std::is_same_v<double, T> || std::numeric_limits<double>::is_iec559)
@@ -190,14 +160,14 @@ void write(iobuf& out, T t) {
     static_assert(are_bytes_and_string_different<Type>);
     static_assert(has_serde_write<Type> || is_serde_compatible_v<Type>);
 
-    if constexpr (is_envelope_v<Type>) {
+    if constexpr (is_envelope<Type>) {
         write(out, Type::redpanda_serde_version);
         write(out, Type::redpanda_serde_compat_version);
 
         auto size_placeholder = out.reserve(sizeof(serde_size_t));
 
         auto checksum_placeholder = iobuf::placeholder{};
-        if constexpr (is_checksum_envelope_v<Type>) {
+        if constexpr (is_checksum_envelope<Type>) {
             checksum_placeholder = out.reserve(sizeof(checksum_t));
         }
 
@@ -218,7 +188,7 @@ void write(iobuf& out, T t) {
         size_placeholder.write(
           reinterpret_cast<char const*>(&size), sizeof(serde_size_t));
 
-        if constexpr (is_checksum_envelope_v<Type>) {
+        if constexpr (is_checksum_envelope<Type>) {
             auto crc = crc::crc32c{};
             auto in = iobuf_const_parser{out};
             in.skip(size_before);
@@ -388,7 +358,7 @@ header read_header(iobuf_parser& in, std::size_t const bytes_left_limit) {
     auto const size = read_nested<serde_size_t>(in, bytes_left_limit);
 
     auto checksum = checksum_t{};
-    if constexpr (is_checksum_envelope_v<T>) {
+    if constexpr (is_checksum_envelope<T>) {
         checksum = read_nested<checksum_t>(in, bytes_left_limit);
     }
 
@@ -443,10 +413,10 @@ void read_nested(iobuf_parser& in, T& t, std::size_t const bytes_left_limit) {
     static_assert(are_bytes_and_string_different<Type>);
     static_assert(has_serde_read<T> || is_serde_compatible_v<Type>);
 
-    if constexpr (is_envelope_v<Type>) {
+    if constexpr (is_envelope<Type>) {
         auto const h = read_header<Type>(in, bytes_left_limit);
 
-        if constexpr (is_checksum_envelope_v<Type>) {
+        if constexpr (is_checksum_envelope<Type>) {
             auto const shared = in.share(in.bytes_left() - h._bytes_left_limit);
             auto read_only_in = iobuf_const_parser{shared};
             auto crc = crc::crc32c{};
@@ -695,7 +665,7 @@ ss::future<std::decay_t<T>> read_async(iobuf_parser& in) {
 template<typename T>
 ss::future<> write_async(iobuf& out, T t) {
     using Type = std::decay_t<T>;
-    if constexpr (is_envelope_v<Type> && has_serde_async_write<Type>) {
+    if constexpr (is_envelope<Type> && has_serde_async_write<Type>) {
         write(out, Type::redpanda_serde_version);
         write(out, Type::redpanda_serde_compat_version);
 

src/v/serde/test/fuzz.cc

@@ -18,12 +18,7 @@ bool eq(
     return ((std::get<I>(a) == std::get<I>(b)) && ...);
 }
 
-template<
-  typename T1,
-  typename T2,
-  typename std::enable_if_t<
-    serde::is_envelope_v<T1> && serde::is_envelope_v<T2>,
-    void*> = nullptr>
+template<serde::is_envelope T1, serde::is_envelope T2>
 bool operator==(T1 const& a, T2 const& b) {
     return eq(
       envelope_to_tuple(a),
@@ -69,7 +64,7 @@ void init(
   data_gen& gen,
   std::index_sequence<Generation...> generations,
   int depth = 0) {
-    if constexpr (serde::is_envelope_v<T>) {
+    if constexpr (serde::is_envelope<T>) {
         ((std::apply(
            [&](auto&&... args) {
                (init(args, gen, generations, depth + 1), ...);

src/v/serde/test/serde_test.cc

@@ -90,10 +90,10 @@ struct test_msg1_new_manual {
 };
 
 struct not_an_envelope {};
-static_assert(!serde::is_envelope_v<not_an_envelope>);
-static_assert(serde::is_envelope_v<test_msg1>);
-static_assert(serde::inherits_from_envelope_v<test_msg1_new>);
-static_assert(!serde::inherits_from_envelope_v<test_msg1_new_manual>);
+static_assert(!serde::is_envelope<not_an_envelope>);
+static_assert(serde::is_envelope<test_msg1>);
+static_assert(serde::inherits_from_envelope<test_msg1_new>);
+static_assert(!serde::inherits_from_envelope<test_msg1_new_manual>);
 static_assert(test_msg1::redpanda_serde_version == 4);
 static_assert(test_msg1::redpanda_serde_compat_version == 0);
 
@@ -234,7 +234,7 @@ struct complex_msg : serde::envelope<complex_msg, serde::version<3>> {
     int32_t _x;
 };
 
-static_assert(serde::is_envelope_v<complex_msg>);
+static_assert(serde::is_envelope<complex_msg>);
 
 SEASTAR_THREAD_TEST_CASE(complex_msg_test) {
     auto b = iobuf();
@@ -386,7 +386,7 @@ struct test_snapshot_header
     int32_t metadata_size;
 };
 
-static_assert(serde::is_envelope_v<test_snapshot_header>);
+static_assert(serde::is_envelope<test_snapshot_header>);
 static_assert(serde::has_serde_async_read<test_snapshot_header>);
 static_assert(serde::has_serde_async_write<test_snapshot_header>);