From f82e5bb2008e03aa082b851b79339b4f438e7811 Mon Sep 17 00:00:00 2001
From: Ben Deane <ben@elbeno.com>
Date: Mon, 2 Dec 2024 10:48:24 -0700
Subject: [PATCH] :sparkles: Add message packing

Problem:
- Message definitions can't easily be combined, especially when the locations of
  fields are fixed in definitions and it would be useful to pack two or more
  messages together.
- See issue #653

Solution:
- Add primitives for combining messages:
  - `shifted_by` on fields and messages to increment their location(s)
  - `msg::combine` to naively combine multiple messages together
  - `msg::pack` to concatenate messages according to desired alignment

Notes:
- `at` has been simplified
---
 docs/message.adoc       |  48 +++++++++++++++++++
 include/msg/field.hpp   | 103 ++++++++++++++--------------------------
 include/msg/message.hpp |  50 +++++++++++++++++++
 test/msg/message.cpp    |  83 ++++++++++++++++++++++++++++++++
 4 files changed, 216 insertions(+), 68 deletions(-)

diff --git a/docs/message.adoc b/docs/message.adoc
index 265aa5a0..df502348 100644
--- a/docs/message.adoc
+++ b/docs/message.adoc
@@ -136,6 +136,54 @@ using msg1_defn = extend<header_defn, "msg", type_f::with_required<1>, payload_f
 using msg2_defn = extend<header_defn, "msg", type_f::with_required<2>, payload_f>;
 ----
 
+==== Combining and packing messages
+
+It is sometimes useful to combine multiple message definitions, to avoid
+repetition. For example, adding a payload message to a header:
+
+[source,cpp]
+----
+using type_f = field<"type", std::uint32_t>::located<at{0_dw, 7_msb, 0_lsb}>;
+using header_defn = message<"header", type_f>;
+
+using data_f = field<"data">, std::uint32_t>::located<at{0_dw, 15_msb, 7_lsb}>;
+using payload_defn = message<"payload", data_f>;
+
+using msg_defn = extend<
+    combine<"msg", header_defn, payload_defn>,
+    type_f::with_required<1>>;
+----
+
+The combined definition incorporates all the fields of the messages. And as
+shown, the combination might typically be `extend`ed with a constraint on the
+header field.
+
+Other times it is useful to automatically concatenate or `pack` messages
+together, where the field locations in each message start at 0.
+
+[source,cpp]
+----
+using type_f = field<"type", std::uint32_t>::located<at{0_dw, 5_msb, 0_lsb}>;
+using header_defn = message<"header", type_f>;
+
+// note: data_f collides with type_f under a naive combination
+using data_f = field<"data">, std::uint32_t>::located<at{0_dw, 7_msb, 0_lsb}>;
+using payload_defn = message<"payload", data_f>;
+
+using msg_defn = extend<
+    pack<"msg", std::uint8_t, header_defn, payload_defn>,
+    type_f::with_required<1>>;
+
+// resulting message layout:
+// byte   0        1
+// bit    01234567 01234567
+// field  |type|xx |data  |
+----
+
+The second parameter to `pack` (`std::uint8_t` in the example above) defines how
+the messages are packed together - in this case, each subsequent message is
+byte-aligned.
+
 ==== Owning vs view types
 
 An owning message uses underlying storage: by default, this is a `std::array` of
diff --git a/include/msg/field.hpp b/include/msg/field.hpp
index 4ea8d070..557c251c 100644
--- a/include/msg/field.hpp
+++ b/include/msg/field.hpp
@@ -64,15 +64,6 @@ template <typename T, typename Spec>
 concept field_locator_for =
     field_extractor_for<T, Spec> and field_inserter_for<T, Spec>;
 
-template <typename T>
-concept field_location = requires(T const &t) {
-    { t.index() } -> std::same_as<std::uint32_t>;
-    { t.lsb() } -> std::same_as<std::uint32_t>;
-    { t.size() } -> std::same_as<std::uint32_t>;
-    { t.valid() } -> std::same_as<bool>;
-    { t.sort_key() } -> std::same_as<std::uint32_t>;
-};
-
 namespace detail {
 template <typename Name, typename T, std::uint32_t BitSize>
 struct field_spec_t {
@@ -305,62 +296,33 @@ CONSTEVAL auto operator""_msb(unsigned long long int v) -> msb_t {
 }
 } // namespace literals
 
-template <typename...> struct at;
-
-template <> struct at<dword_index_t, msb_t, lsb_t> {
-    dword_index_t index_{};
-    msb_t msb_{};
-    lsb_t lsb_{};
+struct bit_unit {};
 
-    [[nodiscard]] constexpr auto
-    index() const -> std::underlying_type_t<dword_index_t> {
-        return stdx::to_underlying(index_);
-    }
-    [[nodiscard]] constexpr auto lsb() const -> std::underlying_type_t<lsb_t> {
-        return stdx::to_underlying(lsb_);
-    }
-    [[nodiscard]] constexpr auto size() const -> std::underlying_type_t<lsb_t> {
-        return stdx::to_underlying(msb_) - lsb() + 1;
-    }
-    [[nodiscard]] constexpr auto valid() const -> bool {
-        return size() <= 64 and
-               stdx::to_underlying(msb_) >= stdx::to_underlying(lsb_);
-    }
-    [[nodiscard]] constexpr auto
-    sort_key() const -> std::underlying_type_t<lsb_t> {
-        return index() * 32u + stdx::to_underlying(lsb_);
+template <typename Unit, typename T>
+constexpr auto unit_bit_size(T t) -> std::uint32_t {
+    if constexpr (std::same_as<Unit, bit_unit>) {
+        return static_cast<std::uint32_t>(t);
+    } else {
+        return static_cast<std::uint32_t>(t) * sizeof(Unit) * CHAR_BIT;
     }
-};
+}
 
-template <> struct at<byte_index_t, msb_t, lsb_t> {
-    byte_index_t index_{};
+struct at {
     msb_t msb_{};
     lsb_t lsb_{};
 
-    [[nodiscard]] constexpr auto
-    index() const -> std::underlying_type_t<byte_index_t> {
-        return stdx::to_underlying(index_) / 4;
-    }
-    [[nodiscard]] constexpr auto lsb() const -> std::underlying_type_t<lsb_t> {
-        return (stdx::to_underlying(index_) * 8) % 32 +
-               stdx::to_underlying(lsb_);
-    }
-    [[nodiscard]] constexpr auto size() const -> std::underlying_type_t<lsb_t> {
-        return stdx::to_underlying(msb_) - stdx::to_underlying(lsb_) + 1;
-    }
-    [[nodiscard]] constexpr auto valid() const -> bool {
-        return size() <= 8 and
-               stdx::to_underlying(msb_) >= stdx::to_underlying(lsb_);
-    }
-    [[nodiscard]] constexpr auto
-    sort_key() const -> std::underlying_type_t<lsb_t> {
-        return index() * 32u + stdx::to_underlying(lsb_);
-    }
-};
-
-template <> struct at<msb_t, lsb_t> {
-    msb_t msb_{};
-    lsb_t lsb_{};
+    constexpr at() = default;
+    constexpr at(msb_t m, lsb_t l) : msb_{m}, lsb_{l} {}
+    constexpr at(dword_index_t di, msb_t m, lsb_t l)
+        : msb_{unit_bit_size<std::uint32_t>(stdx::to_underlying(di)) +
+               stdx::to_underlying(m)},
+          lsb_{unit_bit_size<std::uint32_t>(stdx::to_underlying(di)) +
+               stdx::to_underlying(l)} {}
+    constexpr at(byte_index_t bi, msb_t m, lsb_t l)
+        : msb_{unit_bit_size<std::uint8_t>(stdx::to_underlying(bi)) +
+               stdx::to_underlying(m)},
+          lsb_{unit_bit_size<std::uint8_t>(stdx::to_underlying(bi)) +
+               stdx::to_underlying(l)} {}
 
     [[nodiscard]] constexpr auto
     index() const -> std::underlying_type_t<lsb_t> {
@@ -380,13 +342,14 @@ template <> struct at<msb_t, lsb_t> {
     sort_key() const -> std::underlying_type_t<lsb_t> {
         return stdx::to_underlying(lsb_);
     }
+    [[nodiscard]] constexpr auto shifted_by(auto n) const -> at {
+        return {msb_t{stdx::to_underlying(msb_) + n},
+                lsb_t{stdx::to_underlying(lsb_) + n}};
+    }
 };
 
-template <typename... Ts> at(Ts...) -> at<Ts...>;
-
 namespace detail {
-template <auto... Ats>
-    requires(... and field_location<decltype(Ats)>)
+template <at... Ats>
 using locator_for =
     field_locator_t<bits_locator_t<Ats.index(), Ats.size(), Ats.lsb()>...>;
 
@@ -428,9 +391,8 @@ struct field_id_t {};
 
 template <typename Name, typename T = std::uint32_t,
           typename Default = detail::with_default<T>,
-          match::matcher M = match::always_t, auto... Ats>
-    requires std::is_trivially_copyable_v<T> and
-                 (... and field_location<decltype(Ats)>)
+          match::matcher M = match::always_t, at... Ats>
+    requires std::is_trivially_copyable_v<T>
 class field_t : public field_spec_t<Name, T, detail::field_size<Ats...>>,
                 public detail::locator_for<Ats...>,
                 public Default {
@@ -543,6 +505,12 @@ class field_t : public field_spec_t<Name, T, detail::field_size<Ats...>>,
     using with_required = field_t<Name, T, detail::with_const_default<T, V>,
                                   msg::equal_to_t<field_t, V>, Ats...>;
 
+    // ======================================================================
+    // shift a field
+    template <auto N, typename Unit = bit_unit>
+    using shifted_by =
+        field_t<Name, T, Default, M, Ats.shifted_by(unit_bit_size<Unit>(N))...>;
+
     // ======================================================================
     // legacy aliases
     template <typename X> using WithMatch = with_matcher<X>;
@@ -560,8 +528,7 @@ template <stdx::ct_string Name, typename T = std::uint32_t,
           typename Default = detail::with_default<T>,
           match::matcher M = match::always_t>
 struct field {
-    template <auto... Ats>
-        requires(... and field_location<decltype(Ats)>)
+    template <at... Ats>
     using located = detail::field_t<
         decltype(stdx::ct_string_to_type<Name, sc::string_constant>()), T,
         Default, M, Ats...>;
diff --git a/include/msg/message.hpp b/include/msg/message.hpp
index 3b27afc1..53700636 100644
--- a/include/msg/message.hpp
+++ b/include/msg/message.hpp
@@ -19,6 +19,7 @@
 #include <stdx/utility.hpp>
 
 #include <boost/mp11/algorithm.hpp>
+#include <boost/mp11/function.hpp>
 #include <boost/mp11/list.hpp>
 #include <boost/mp11/set.hpp>
 
@@ -339,6 +340,10 @@ template <stdx::ct_string Name, typename... Fields> struct message {
     template <template <typename, std::size_t> typename C, typename T>
     using custom_storage_t = typename access_t::template storage_t<C, T>;
 
+    template <auto N, typename Unit = bit_unit>
+    using shifted_by =
+        message<Name, typename Fields::template shifted_by<N, Unit>...>;
+
     template <typename S>
     constexpr static auto fits_inside =
         (... and Fields::template fits_inside<S>());
@@ -652,4 +657,49 @@ call_with_message(F &&f, S &&s, Args &&...args) -> decltype(auto) {
                                   std::forward<Args>(args)...);
     }
 }
+
+namespace detail {
+template <typename AlignTo, typename... Msgs>
+using msg_sizes = stdx::type_list<typename Msgs::template size<AlignTo>...>;
+
+template <typename S>
+using negated_size = std::integral_constant<std::size_t, -S::value>;
+
+template <typename AlignTo, typename... Msgs>
+using msg_offsets = boost::mp11::mp_partial_sum<
+    msg_sizes<AlignTo, Msgs...>,
+    negated_size<typename boost::mp11::mp_first<
+        stdx::type_list<Msgs...>>::template size<AlignTo>>,
+    boost::mp11::mp_plus>;
+
+template <typename AlignTo> struct shift_msg_q {
+    template <typename Offset, typename Msg>
+    using fn = typename Msg::template shifted_by<Offset::value, AlignTo>;
+};
+
+template <typename AlignTo, typename... Msgs>
+using shifted_msgs = boost::mp11::mp_transform_q<shift_msg_q<AlignTo>,
+                                                 msg_offsets<AlignTo, Msgs...>,
+                                                 stdx::type_list<Msgs...>>;
+
+template <stdx::ct_string Name> struct combiner {
+    template <typename... Fields> using fn = msg::message<Name, Fields...>;
+};
+
+template <stdx::ct_string Name> struct combine_q {
+    template <typename... Msgs>
+        requires(sizeof...(Msgs) > 0)
+    using fn = boost::mp11::mp_apply_q<
+        combiner<Name>, boost::mp11::mp_append<typename Msgs::fields_t...>>;
+};
+} // namespace detail
+
+template <stdx::ct_string Name, typename... Msgs>
+    requires(sizeof...(Msgs) > 0)
+using combine = typename detail::combine_q<Name>::template fn<Msgs...>;
+
+template <stdx::ct_string Name, typename AlignTo, typename... Msgs>
+    requires(sizeof...(Msgs) > 0)
+using pack = boost::mp11::mp_apply_q<detail::combine_q<Name>,
+                                     detail::shifted_msgs<AlignTo, Msgs...>>;
 } // namespace msg
diff --git a/test/msg/message.cpp b/test/msg/message.cpp
index d8f83589..e7cb5bc7 100644
--- a/test/msg/message.cpp
+++ b/test/msg/message.cpp
@@ -538,3 +538,86 @@ TEST_CASE("message equivalence matcher", "[message]") {
                         "f3"_field = 0xd00d};
     CHECK_THAT(m2, not MsgEquiv(m));
 }
+
+TEST_CASE("message reports size", "[message]") {
+    static_assert(msg_defn::size<std::uint32_t>::value == 2);
+    static_assert(msg_defn::size<std::uint16_t>::value == 4);
+    static_assert(msg_defn::size<std::uint8_t>::value == 7);
+}
+
+TEST_CASE("shift a field by bit offset", "[message]") {
+    using new_field = id_field::shifted_by<3>;
+    using expected_field =
+        field<"id", std::uint32_t>::located<at{34_msb, 27_lsb}>;
+    static_assert(std::is_same_v<new_field, expected_field>);
+}
+
+TEST_CASE("shift a field by byte offset", "[message]") {
+    using new_field = id_field::shifted_by<1, std::uint8_t>;
+    using expected_field =
+        field<"id", std::uint32_t>::located<at{39_msb, 32_lsb}>;
+    static_assert(std::is_same_v<new_field, expected_field>);
+}
+
+TEST_CASE("shift all fields in a message", "[message]") {
+    using base_defn = message<"msg", id_field, field1>;
+    using defn = base_defn::shifted_by<1, std::uint8_t>;
+
+    using shifted_id = id_field::shifted_by<1, std::uint8_t>;
+    using shifted_field1 = field1::shifted_by<1, std::uint8_t>;
+    using expected_defn = message<"msg", shifted_id, shifted_field1>;
+    static_assert(std::is_same_v<defn, expected_defn>);
+}
+
+TEST_CASE("combine messages", "[message]") {
+    using f1 = field<"f1", std::uint32_t>::located<at{23_msb, 16_lsb}>;
+    using f2 = field<"f2", std::uint32_t>::located<at{15_msb, 0_lsb}>;
+    using m1 = message<"m1", f1, f2>;
+
+    using f3 = field<"f3", std::uint32_t>::located<at{23_msb, 16_lsb}>;
+    using f4 = field<"f4", std::uint32_t>::located<at{15_msb, 0_lsb}>;
+    using m2 = message<"m2", f3, f4>;
+
+    using defn = combine<"defn", m1, m2::shifted_by<1, std::uint32_t>>;
+    using expected_defn =
+        message<"defn", f1, f2, f3::shifted_by<1, std::uint32_t>,
+                f4::shifted_by<1, std::uint32_t>>;
+    static_assert(std::is_same_v<defn, expected_defn>);
+}
+
+TEST_CASE("combine 1 message", "[message]") {
+    using f1 = field<"f1", std::uint32_t>::located<at{15_msb, 0_lsb}>;
+    using f2 = field<"f2", std::uint32_t>::located<at{23_msb, 16_lsb}>;
+    using m1 = message<"m1", f1, f2>;
+
+    using defn = combine<"defn", m1>;
+    using expected_defn = message<"defn", f1, f2>;
+    static_assert(std::is_same_v<defn, expected_defn>);
+}
+
+TEST_CASE("pack messages", "[message]") {
+    using f1 = field<"f1", std::uint32_t>::located<at{15_msb, 0_lsb}>;
+    using f2 = field<"f2", std::uint32_t>::located<at{23_msb, 16_lsb}>;
+    using m1 = message<"m1", f1, f2>;
+
+    using f3 = field<"f3", std::uint32_t>::located<at{15_msb, 0_lsb}>;
+    using f4 = field<"f4", std::uint32_t>::located<at{23_msb, 16_lsb}>;
+    using m2 = message<"m2", f3, f4>;
+
+    using defn = pack<"defn", std::uint8_t, m1, m2>;
+    using expected_defn =
+        message<"defn", f1, f2,
+                f3::shifted_by<m1::size<std::uint8_t>::value, std::uint8_t>,
+                f4::shifted_by<m1::size<std::uint8_t>::value, std::uint8_t>>;
+    static_assert(std::is_same_v<defn, expected_defn>);
+}
+
+TEST_CASE("pack 1 message", "[message]") {
+    using f1 = field<"f1", std::uint32_t>::located<at{15_msb, 0_lsb}>;
+    using f2 = field<"f2", std::uint32_t>::located<at{23_msb, 16_lsb}>;
+    using m1 = message<"m1", f1, f2>;
+
+    using defn = pack<"defn", std::uint8_t, m1>;
+    using expected_defn = message<"defn", f1, f2>;
+    static_assert(std::is_same_v<defn, expected_defn>);
+}