Merge pull request #762 from elbeno/test-msg

🐛 Match fields by name after packing

Author: bdeane-intel

docs/message.adoc

@@ -210,6 +210,34 @@ 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.
 
+CAUTION: After combining or packing messages, the fields inside them may have
+moved!
+
+Any matchers defined on the original fields may cause problems when matching
+against raw data, because they will be looking in the wrong place. (Matching
+when the message type is known is OK, because the field is resolved by its
+name.)
+
+To avoid matcher problems, define matchers after combining or packing messages.
+To help with this, use the `field_t` alias on the message definition if needed.
+
+[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>;
+
+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>>;
+
+// msg_defn does not contain data_f because packing moved it
+// but we can get the actual data field by name, if we need it
+using new_data_f = msg_defn::field_t<"data">;
+----
+
 ==== Owning vs view types
 
 An owning message uses underlying storage: by default, this is a `std::array` of

include/msg/field_matchers.hpp

@@ -186,7 +186,7 @@ struct rel_matcher_t {
         if constexpr (stdx::range<Msg>) {
             return Field::extract(msg);
         } else {
-            return Field::extract(std::data(msg));
+            return msg.get(Field{});
         }
     }
 };

include/msg/message.hpp

@@ -209,29 +209,27 @@ template <stdx::ct_string Name, typename... Fields> class msg_access {
 
     template <stdx::range R, some_field_value V>
     constexpr static auto set1(R &&r, V v) -> void {
-        using Field = std::remove_cvref_t<decltype(stdx::get<name_for<V>>(
-            FieldsTuple{}))>;
+        using Field = field_t<name_for<V>>;
         check<Field, std::remove_cvref_t<R>>();
         Field::insert(std::forward<R>(r),
                       static_cast<typename Field::value_type>(v.value));
     }
 
     template <typename N, stdx::range R>
     constexpr static auto set_default(R &&r) -> void {
-        using Field =
-            std::remove_cvref_t<decltype(stdx::get<N>(FieldsTuple{}))>;
-        check<Field, std::remove_cvref_t<R>>();
-        Field::insert_default(std::forward<R>(r));
+        check<field_t<N>, std::remove_cvref_t<R>>();
+        field_t<N>::insert_default(std::forward<R>(r));
     }
 
     template <typename N, stdx::range R> constexpr static auto get(R &&r) {
-        using Field =
-            std::remove_cvref_t<decltype(stdx::get<N>(FieldsTuple{}))>;
-        check<Field, std::remove_cvref_t<R>>();
-        return Field::extract(std::forward<R>(r));
+        check<field_t<N>, std::remove_cvref_t<R>>();
+        return field_t<N>::extract(std::forward<R>(r));
     }
 
   public:
+    template <typename N>
+    using field_t = std::remove_cvref_t<decltype(stdx::get<N>(FieldsTuple{}))>;
+
     template <stdx::range R, stdx::ct_string... Ns>
     constexpr static auto set(R &&r, field_name<Ns>...) -> void {
         (set_default<Ns>(r), ...);
@@ -369,6 +367,9 @@ struct message {
         std::integral_constant<std::size_t,
                                detail::storage_size<Fields...>::template in<T>>;
 
+    template <stdx::ct_string N>
+    using field_t = typename access_t::template field_t<stdx::cts_t<N>>;
+
     template <template <typename, std::size_t> typename C, typename T>
     using custom_storage_t = typename access_t::template storage_t<C, T>;
 

test/msg/message.cpp

@@ -33,6 +33,11 @@ template <>
 inline auto logging::config<> =
     logging::fmt::config{std::back_inserter(log_buffer)};
 
+TEST_CASE("get a field from a message", "[message]") {
+    using F = msg_defn::field_t<"f1">;
+    STATIC_REQUIRE(std::is_same_v<F, field1>);
+}
+
 TEST_CASE("message with automatic storage", "[message]") {
     test_msg msg{};
     auto data = msg.data();
@@ -683,6 +688,56 @@ TEST_CASE("pack with empty messages", "[message]") {
     STATIC_REQUIRE(std::is_same_v<defn, expected_defn>);
 }
 
+TEST_CASE("field matchers work with packed 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>;
+
+    constexpr auto m = msg::equal_to_t<f3, 3>{};
+    using defn = pack<"defn", std::uint8_t, m1, m2>;
+    constexpr owning<defn> msg{"f1"_field = 1, "f2"_field = 2, "f3"_field = 3,
+                               "f4"_field = 4};
+    STATIC_REQUIRE(m(msg));
+}
+
+TEST_CASE("a packed message matches itself", "[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::with_equal_to<3>, f4>;
+
+    using defn = pack<"defn", std::uint8_t, m1, m2>;
+    constexpr owning<defn> msg{"f1"_field = 1, "f2"_field = 2, "f3"_field = 3,
+                               "f4"_field = 4};
+    STATIC_REQUIRE(defn::matcher_t{}(msg));
+}
+
+TEST_CASE("correct field matchers can be post-defined", "[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>;
+    constexpr owning<defn> msg{"f1"_field = 1, "f2"_field = 2, "f3"_field = 3,
+                               "f4"_field = 4};
+
+    constexpr auto m_wrong = msg::equal_to_t<f3, 3>{};
+    STATIC_REQUIRE(not m_wrong(msg.data()));
+    constexpr auto m_right = msg::equal_to_t<defn::field_t<"f3">, 3>{};
+    STATIC_REQUIRE(m_right(msg.data()));
+}
+
 namespace {
 [[maybe_unused]] constexpr inline struct custom_t {
     template <typename T>