🐛 Allow 64-bit values in MIPI arg encoding

cmake/string_catalog.cmake

@@ -25,7 +25,7 @@ function(gen_str_catalog)
         if(${lib_type} STREQUAL OBJECT_LIBRARY)
             add_custom_command(
                 OUTPUT ${UNDEF}
-                DEPENDS ${LIB}
+                DEPENDS $<TARGET_OBJECTS:${LIB}>
                 COMMAND ${CMAKE_NM} -uC "$<TARGET_OBJECTS:${LIB}>" > "${UNDEF}"
                 COMMAND_EXPAND_LISTS)
         else()
@@ -82,6 +82,7 @@ function(gen_str_catalog)
             ${GUID_MASK_ARG} ${MODULE_ID_MAX_ARG}
         DEPENDS ${UNDEFS} ${INPUT_JSON} ${SC_GEN_STR_CATALOG} ${STABLE_JSON}
         COMMAND_EXPAND_LISTS)
+
     if(SC_OUTPUT_LIB)
         add_library(${SC_OUTPUT_LIB} STATIC ${SC_OUTPUT_CPP})
         target_link_libraries(${SC_OUTPUT_LIB} PUBLIC cib)

include/log/catalog/catalog.hpp

@@ -15,3 +15,8 @@ using module_id = std::uint32_t;
 
 template <typename> extern auto catalog() -> string_id;
 template <typename> extern auto module() -> module_id;
+
+struct encode_32;
+struct encode_64;
+struct encode_u32;
+struct encode_u64;

include/log/catalog/mipi_builder.hpp

@@ -4,6 +4,7 @@
 #include <log/catalog/mipi_messages.hpp>
 
 #include <stdx/compiler.hpp>
+#include <stdx/type_traits.hpp>
 #include <stdx/utility.hpp>
 
 #include <array>
@@ -13,89 +14,111 @@
 #include <utility>
 
 namespace logging::mipi {
+template <typename T>
+concept signed_packable =
+    std::signed_integral<T> and sizeof(T) <= sizeof(std::int64_t);
+
+template <typename T>
+concept unsigned_packable =
+    std::unsigned_integral<T> and sizeof(T) <= sizeof(std::int64_t);
+
+template <typename T>
+concept enum_packable = std::is_enum_v<T> and sizeof(T) <= sizeof(std::int32_t);
+
+template <typename T>
+concept packable =
+    signed_packable<T> or unsigned_packable<T> or enum_packable<T>;
+
+template <typename T> struct encoding;
+
+template <signed_packable T> struct encoding<T> {
+    using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
+                                         encode_32, encode_64>;
+    using pack_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
+                                       std::int32_t, std::int64_t>;
+};
+
+template <unsigned_packable T> struct encoding<T> {
+    using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
+                                         encode_u32, encode_u64>;
+    using pack_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
+                                       std::uint32_t, std::uint64_t>;
+};
+
+template <enum_packable T>
+struct encoding<T> : encoding<stdx::underlying_type_t<T>> {};
+
+template <packable T> using pack_as_t = typename encoding<T>::pack_t;
+template <packable T> using encode_as_t = typename encoding<T>::encode_t;
+
 template <typename> struct builder;
 
 template <> struct builder<defn::short32_msg_t> {
-    template <auto Level, std::same_as<std::uint32_t>... Ts>
-    static auto build(string_id id, module_id, Ts...) {
+    template <auto Level> static auto build(string_id id, module_id) {
         using namespace msg;
         return owning<defn::short32_msg_t>{"payload"_field = id};
     }
 };
 
 template <typename Storage> struct catalog_builder {
-    template <auto Level, std::same_as<std::uint32_t>... Ts>
-    static auto build(string_id id, module_id m, Ts... msg_data) {
+    template <auto Level, packable... Ts>
+    static auto build(string_id id, module_id m, Ts... args) {
         using namespace msg;
         defn::catalog_msg_t::owner_t<Storage> message{"severity"_field = Level,
                                                       "module_id"_field = m};
 
-        constexpr auto header_size =
-            defn::catalog_msg_t::size<typename Storage::value_type>::value;
-        constexpr auto copy_arg = [](std::uint32_t arg, auto &dest) {
-            std::memcpy(dest, &arg, sizeof(std::uint32_t));
-            dest += sizeof(std::uint32_t);
-        };
-        auto dest = &message.data()[header_size];
-        copy_arg(stdx::to_le(id), dest);
-        (copy_arg(stdx::to_le(msg_data), dest), ...);
-
-        return message;
-    }
-};
+        using V = typename Storage::value_type;
+        constexpr auto header_size = defn::catalog_msg_t::size<V>::value;
 
-template <typename Storage>
-    requires std::same_as<typename Storage::value_type, std::uint32_t>
-struct catalog_builder<Storage> {
-    template <auto Level, std::same_as<std::uint32_t>... Ts>
-    static auto build(string_id id, module_id m, Ts... msg_data) {
-        using namespace msg;
-        defn::catalog_msg_t::owner_t<Storage> message{"severity"_field = Level,
-                                                      "module_id"_field = m};
+        auto const pack_arg = []<typename T>(V *p, T arg) -> V * {
+            auto const packed = stdx::to_le(stdx::as_unsigned(
+                static_cast<pack_as_t<T>>(stdx::to_underlying(arg))));
+            std::memcpy(p, &packed, sizeof(packed));
+            return p + stdx::sized8{sizeof(packed)}.in<V>();
+        };
 
-        constexpr auto header_size =
-            defn::catalog_msg_t::size<std::uint32_t>::value;
         auto dest = &message.data()[header_size];
-        *dest++ = stdx::to_le(id);
-        ((*dest++ = stdx::to_le(msg_data)), ...);
+        dest = pack_arg(dest, stdx::to_le(id));
+        ((dest = pack_arg(dest, args)), ...);
 
         return message;
     }
 };
 
 template <> struct builder<defn::catalog_msg_t> {
-    template <auto Level, std::same_as<std::uint32_t>... Ts>
-    static auto build(string_id id, module_id m, Ts... msg_data) {
+    template <auto Level, typename... Ts>
+    static auto build(string_id id, module_id m, Ts... args) {
         using namespace msg;
-        if constexpr (sizeof...(Ts) <= 2u) {
+        if constexpr ((0 + ... + sizeof(Ts)) <= sizeof(std::uint32_t) * 2) {
             constexpr auto header_size =
                 defn::catalog_msg_t::size<std::uint32_t>::value;
-            constexpr auto payload_len = 1 + sizeof...(Ts);
+            constexpr auto payload_size =
+                stdx::sized8{(sizeof(id) + ... + sizeof(pack_as_t<Ts>))}
+                    .in<std::uint32_t>();
             using storage_t =
-                std::array<std::uint32_t, header_size + payload_len>;
-            return catalog_builder<storage_t>{}.template build<Level>(
-                id, m, msg_data...);
+                std::array<std::uint32_t, header_size + payload_size>;
+            return catalog_builder<storage_t>{}.template build<Level>(id, m,
+                                                                      args...);
         } else {
             constexpr auto header_size =
                 defn::catalog_msg_t::size<std::uint8_t>::value;
-            constexpr auto payload_len = (sizeof(id) + ... + sizeof(Ts));
+            constexpr auto payload_size = (sizeof(id) + ... + sizeof(Ts));
             using storage_t =
-                std::array<std::uint8_t, header_size + payload_len>;
-            return catalog_builder<storage_t>{}.template build<Level>(
-                id, m, msg_data...);
+                std::array<std::uint8_t, header_size + payload_size>;
+            return catalog_builder<storage_t>{}.template build<Level>(id, m,
+                                                                      args...);
         }
     }
 };
 
 struct default_builder {
-    template <auto Level, std::same_as<std::uint32_t>... Ts>
-    static auto build(string_id id, module_id m, Ts... msg_data) {
+    template <auto Level, packable... Ts>
+    static auto build(string_id id, module_id m, Ts... args) {
         if constexpr (sizeof...(Ts) == 0u) {
-            return builder<defn::short32_msg_t>{}.template build<Level>(
-                id, m, msg_data...);
+            return builder<defn::short32_msg_t>{}.template build<Level>(id, m);
         } else {
             return builder<defn::catalog_msg_t>{}.template build<Level>(
-                id, m, msg_data...);
+                id, m, args...);
         }
     }
 };

include/log/catalog/mipi_encoder.hpp

@@ -26,7 +26,7 @@ template <typename S, typename... Args> constexpr auto to_message() {
     using char_t = typename std::remove_cv_t<decltype(s)>::value_type;
     return [&]<std::size_t... Is>(std::integer_sequence<std::size_t, Is...>) {
         return sc::message<
-            sc::undefined<sc::args<Args...>, char_t, s[Is]...>>{};
+            sc::undefined<sc::args<encode_as_t<Args>...>, char_t, s[Is]...>>{};
     }(std::make_integer_sequence<std::size_t, std::size(s)>{});
 }
 
@@ -54,9 +54,8 @@ template <typename TDestinations> struct log_handler {
             using Module =
                 decltype(detail::to_module<get_module(Env{}).value>());
             auto builder = get_builder(Env{}).value;
-            write(
-                builder.template build<L>(catalog<Message>(), module<Module>(),
-                                          static_cast<std::uint32_t>(args)...));
+            write(builder.template build<L>(catalog<Message>(),
+                                            module<Module>(), args...));
         });
     }
 

test/log/catalog1_lib.cpp

@@ -23,7 +23,9 @@ using log_env1 = logging::make_env_t<logging::get_level, logging::level::TRACE>;
 
 auto log_zero_args() -> void;
 auto log_one_ct_arg() -> void;
-auto log_one_rt_arg() -> void;
+auto log_one_32bit_rt_arg() -> void;
+auto log_one_64bit_rt_arg() -> void;
+auto log_one_formatted_rt_arg() -> void;
 auto log_with_non_default_module_id() -> void;
 auto log_with_fixed_module_id() -> void;
 
@@ -38,9 +40,22 @@ auto log_one_ct_arg() -> void {
         format("B string with {} placeholder"_sc, "one"_sc));
 }
 
-auto log_one_rt_arg() -> void {
+auto log_one_32bit_rt_arg() -> void {
     auto cfg = logging::mipi::config{test_log_args_destination{}};
-    cfg.logger.log_msg<log_env1>(format("C string with {} placeholder"_sc, 1));
+    cfg.logger.log_msg<log_env1>(
+        format("C1 string with {} placeholder"_sc, std::int32_t{1}));
+}
+
+auto log_one_64bit_rt_arg() -> void {
+    auto cfg = logging::mipi::config{test_log_args_destination{}};
+    cfg.logger.log_msg<log_env1>(
+        format("C2 string with {} placeholder"_sc, std::int64_t{1}));
+}
+
+auto log_one_formatted_rt_arg() -> void {
+    auto cfg = logging::mipi::config{test_log_args_destination{}};
+    cfg.logger.log_msg<log_env1>(
+        format("C3 string with {:08x} placeholder"_sc, std::int32_t{1}));
 }
 
 auto log_with_non_default_module_id() -> void {

test/log/catalog2a_lib.cpp

@@ -13,6 +13,10 @@ extern int log_calls;
 namespace {
 struct test_log_args_destination {
     auto log_by_args(std::uint32_t, auto...) -> void { ++log_calls; }
+    template <std::size_t N>
+    auto log_by_buf(stdx::span<std::uint8_t const, N>) const {
+        ++log_calls;
+    }
 };
 
 using log_env2a =
@@ -24,5 +28,6 @@ auto log_two_rt_args() -> void;
 auto log_two_rt_args() -> void {
     auto cfg = logging::mipi::config{test_log_args_destination{}};
     cfg.logger.log_msg<log_env2a>(
-        format("D string with {} and {} placeholder"_sc, 1, 2));
+        format("D string with {} and {} placeholder"_sc, std::uint32_t{1},
+               std::int64_t{2}));
 }

test/log/catalog_app.cpp

@@ -12,7 +12,9 @@ extern int log_calls;
 extern std::uint32_t last_header;
 extern auto log_zero_args() -> void;
 extern auto log_one_ct_arg() -> void;
-extern auto log_one_rt_arg() -> void;
+extern auto log_one_32bit_rt_arg() -> void;
+extern auto log_one_64bit_rt_arg() -> void;
+extern auto log_one_formatted_rt_arg() -> void;
 extern auto log_two_rt_args() -> void;
 extern auto log_rt_enum_arg() -> void;
 extern auto log_with_non_default_module_id() -> void;
@@ -38,10 +40,26 @@ TEST_CASE("log one compile-time argument", "[catalog]") {
     CHECK(last_header >> 4u != 0);
 }
 
-TEST_CASE("log one runtime argument", "[catalog]") {
+TEST_CASE("log one 32-bit runtime argument", "[catalog]") {
     log_calls = 0;
     test_critical_section::count = 0;
-    log_one_rt_arg();
+    log_one_32bit_rt_arg();
+    CHECK(test_critical_section::count == 2);
+    CHECK(log_calls == 1);
+}
+
+TEST_CASE("log one 64-bit runtime argument", "[catalog]") {
+    log_calls = 0;
+    test_critical_section::count = 0;
+    log_one_64bit_rt_arg();
+    CHECK(test_critical_section::count == 2);
+    CHECK(log_calls == 1);
+}
+
+TEST_CASE("log one formatted runtime argument", "[catalog]") {
+    log_calls = 0;
+    test_critical_section::count = 0;
+    log_one_formatted_rt_arg();
     CHECK(test_critical_section::count == 2);
     CHECK(log_calls == 1);
 }
@@ -65,7 +83,7 @@ TEST_CASE("log runtime enum argument", "[catalog]") {
 TEST_CASE("log module ids change", "[catalog]") {
     // subtype 1, severity 7, type 3
     std::uint32_t expected_static = (1u << 24u) | (7u << 4u) | 3u;
-    log_one_rt_arg();
+    log_one_32bit_rt_arg();
     CHECK((last_header & expected_static) == expected_static);
 
     auto default_header = last_header;

test/log/mipi_encoder.cpp

@@ -66,6 +66,7 @@ struct test_conc_policy {
 int num_log_args_calls{};
 
 constexpr auto check = [](auto value, auto expected) {
+    static_assert(std::is_same_v<decltype(value), decltype(expected)>);
     CHECK(value == expected);
 };
 
@@ -129,6 +130,34 @@ using log_env = logging::make_env_t<logging::get_level, logging::level::TRACE>;
 
 template <> inline auto conc::injected_policy<> = test_conc_policy{};
 
+TEST_CASE("argument packing", "[mipi]") {
+    static_assert(
+        std::same_as<logging::mipi::pack_as_t<std::int32_t>, std::int32_t>);
+    static_assert(
+        std::same_as<logging::mipi::pack_as_t<std::uint32_t>, std::uint32_t>);
+    static_assert(
+        std::same_as<logging::mipi::pack_as_t<std::int64_t>, std::int64_t>);
+    static_assert(
+        std::same_as<logging::mipi::pack_as_t<std::uint64_t>, std::uint64_t>);
+    static_assert(std::same_as<logging::mipi::pack_as_t<char>, std::int32_t>);
+    static_assert(
+        std::same_as<logging::mipi::pack_as_t<unsigned char>, std::uint32_t>);
+}
+
+TEST_CASE("argument encoding", "[mipi]") {
+    static_assert(
+        std::same_as<logging::mipi::encode_as_t<std::int32_t>, encode_32>);
+    static_assert(
+        std::same_as<logging::mipi::encode_as_t<std::uint32_t>, encode_u32>);
+    static_assert(
+        std::same_as<logging::mipi::encode_as_t<std::int64_t>, encode_64>);
+    static_assert(
+        std::same_as<logging::mipi::encode_as_t<std::uint64_t>, encode_u64>);
+    static_assert(std::same_as<logging::mipi::encode_as_t<char>, encode_32>);
+    static_assert(
+        std::same_as<logging::mipi::encode_as_t<unsigned char>, encode_u32>);
+}
+
 TEST_CASE("log zero arguments", "[mipi]") {
     CIB_LOG_ENV(logging::get_level, logging::level::TRACE);
     test_critical_section::count = 0;
@@ -138,7 +167,7 @@ TEST_CASE("log zero arguments", "[mipi]") {
     CHECK(test_critical_section::count == 2);
 }
 
-TEST_CASE("log one argument", "[mipi]") {
+TEST_CASE("log one 32-bit argument", "[mipi]") {
     CIB_LOG_ENV(logging::get_level, logging::level::TRACE);
     test_critical_section::count = 0;
     auto cfg = logging::mipi::config{
@@ -147,6 +176,17 @@ TEST_CASE("log one argument", "[mipi]") {
     CHECK(test_critical_section::count == 2);
 }
 
+TEST_CASE("log one 64-bit argument", "[mipi]") {
+    CIB_LOG_ENV(logging::get_level, logging::level::TRACE);
+    test_critical_section::count = 0;
+    auto x = std::uint64_t{0x1234'5678'90ab'cdefull};
+    auto cfg = logging::mipi::config{
+        test_log_args_destination<logging::level::TRACE, 42u, 0x90ab'cdefu,
+                                  0x1234'5678u>{}};
+    cfg.logger.log_msg<log_env>(format("{}"_sc, x));
+    CHECK(test_critical_section::count == 2);
+}
+
 TEST_CASE("log two arguments", "[mipi]") {
     CIB_LOG_ENV(logging::get_level, logging::level::TRACE);
     test_critical_section::count = 0;