diff --git a/cmake/string_catalog.cmake b/cmake/string_catalog.cmake index 3b292ba4..b4f38ac2 100644 --- a/cmake/string_catalog.cmake +++ b/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 $ COMMAND ${CMAKE_NM} -uC "$" > "${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) diff --git a/include/log/catalog/catalog.hpp b/include/log/catalog/catalog.hpp index 05aaf3b4..87703fe6 100644 --- a/include/log/catalog/catalog.hpp +++ b/include/log/catalog/catalog.hpp @@ -15,3 +15,8 @@ using module_id = std::uint32_t; template extern auto catalog() -> string_id; template extern auto module() -> module_id; + +struct encode_32; +struct encode_64; +struct encode_u32; +struct encode_u64; diff --git a/include/log/catalog/mipi_builder.hpp b/include/log/catalog/mipi_builder.hpp index a49cd0a0..0352f051 100644 --- a/include/log/catalog/mipi_builder.hpp +++ b/include/log/catalog/mipi_builder.hpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -13,89 +14,111 @@ #include namespace logging::mipi { +template +concept signed_packable = + std::signed_integral and sizeof(T) <= sizeof(std::int64_t); + +template +concept unsigned_packable = + std::unsigned_integral and sizeof(T) <= sizeof(std::int64_t); + +template +concept enum_packable = std::is_enum_v and sizeof(T) <= sizeof(std::int32_t); + +template +concept packable = + signed_packable or unsigned_packable or enum_packable; + +template struct encoding; + +template struct encoding { + using encode_t = stdx::conditional_t; + using pack_t = stdx::conditional_t; +}; + +template struct encoding { + using encode_t = stdx::conditional_t; + using pack_t = stdx::conditional_t; +}; + +template +struct encoding : encoding> {}; + +template using pack_as_t = typename encoding::pack_t; +template using encode_as_t = typename encoding::encode_t; + template struct builder; template <> struct builder { - template ... Ts> - static auto build(string_id id, module_id, Ts...) { + template static auto build(string_id id, module_id) { using namespace msg; return owning{"payload"_field = id}; } }; template struct catalog_builder { - template ... Ts> - static auto build(string_id id, module_id m, Ts... msg_data) { + template + static auto build(string_id id, module_id m, Ts... args) { using namespace msg; defn::catalog_msg_t::owner_t message{"severity"_field = Level, "module_id"_field = m}; - constexpr auto header_size = - defn::catalog_msg_t::size::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::value; -template - requires std::same_as -struct catalog_builder { - template ... Ts> - static auto build(string_id id, module_id m, Ts... msg_data) { - using namespace msg; - defn::catalog_msg_t::owner_t message{"severity"_field = Level, - "module_id"_field = m}; + auto const pack_arg = [](V *p, T arg) -> V * { + auto const packed = stdx::to_le(stdx::as_unsigned( + static_cast>(stdx::to_underlying(arg)))); + std::memcpy(p, &packed, sizeof(packed)); + return p + stdx::sized8{sizeof(packed)}.in(); + }; - constexpr auto header_size = - defn::catalog_msg_t::size::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 { - template ... Ts> - static auto build(string_id id, module_id m, Ts... msg_data) { + template + 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::value; - constexpr auto payload_len = 1 + sizeof...(Ts); + constexpr auto payload_size = + stdx::sized8{(sizeof(id) + ... + sizeof(pack_as_t))} + .in(); using storage_t = - std::array; - return catalog_builder{}.template build( - id, m, msg_data...); + std::array; + return catalog_builder{}.template build(id, m, + args...); } else { constexpr auto header_size = defn::catalog_msg_t::size::value; - constexpr auto payload_len = (sizeof(id) + ... + sizeof(Ts)); + constexpr auto payload_size = (sizeof(id) + ... + sizeof(Ts)); using storage_t = - std::array; - return catalog_builder{}.template build( - id, m, msg_data...); + std::array; + return catalog_builder{}.template build(id, m, + args...); } } }; struct default_builder { - template ... Ts> - static auto build(string_id id, module_id m, Ts... msg_data) { + template + static auto build(string_id id, module_id m, Ts... args) { if constexpr (sizeof...(Ts) == 0u) { - return builder{}.template build( - id, m, msg_data...); + return builder{}.template build(id, m); } else { return builder{}.template build( - id, m, msg_data...); + id, m, args...); } } }; diff --git a/include/log/catalog/mipi_encoder.hpp b/include/log/catalog/mipi_encoder.hpp index dc092c98..f06f2df0 100644 --- a/include/log/catalog/mipi_encoder.hpp +++ b/include/log/catalog/mipi_encoder.hpp @@ -26,7 +26,7 @@ template constexpr auto to_message() { using char_t = typename std::remove_cv_t::value_type; return [&](std::integer_sequence) { return sc::message< - sc::undefined, char_t, s[Is]...>>{}; + sc::undefined...>, char_t, s[Is]...>>{}; }(std::make_integer_sequence{}); } @@ -54,9 +54,8 @@ template struct log_handler { using Module = decltype(detail::to_module()); auto builder = get_builder(Env{}).value; - write( - builder.template build(catalog(), module(), - static_cast(args)...)); + write(builder.template build(catalog(), + module(), args...)); }); } diff --git a/test/log/catalog1_lib.cpp b/test/log/catalog1_lib.cpp index 5f99cdb9..95dc1e9c 100644 --- a/test/log/catalog1_lib.cpp +++ b/test/log/catalog1_lib.cpp @@ -23,7 +23,9 @@ using log_env1 = logging::make_env_t; 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(format("C string with {} placeholder"_sc, 1)); + cfg.logger.log_msg( + 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( + 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( + format("C3 string with {:08x} placeholder"_sc, std::int32_t{1})); } auto log_with_non_default_module_id() -> void { diff --git a/test/log/catalog2a_lib.cpp b/test/log/catalog2a_lib.cpp index b19b5f05..eb441442 100644 --- a/test/log/catalog2a_lib.cpp +++ b/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 + auto log_by_buf(stdx::span) 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( - format("D string with {} and {} placeholder"_sc, 1, 2)); + format("D string with {} and {} placeholder"_sc, std::uint32_t{1}, + std::int64_t{2})); } diff --git a/test/log/catalog_app.cpp b/test/log/catalog_app.cpp index a4c822d0..105d9dc9 100644 --- a/test/log/catalog_app.cpp +++ b/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; diff --git a/test/log/mipi_encoder.cpp b/test/log/mipi_encoder.cpp index 7ca3f7bd..b96b781b 100644 --- a/test/log/mipi_encoder.cpp +++ b/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); CHECK(value == expected); }; @@ -129,6 +130,34 @@ using log_env = logging::make_env_t; template <> inline auto conc::injected_policy<> = test_conc_policy{}; +TEST_CASE("argument packing", "[mipi]") { + static_assert( + std::same_as, std::int32_t>); + static_assert( + std::same_as, std::uint32_t>); + static_assert( + std::same_as, std::int64_t>); + static_assert( + std::same_as, std::uint64_t>); + static_assert(std::same_as, std::int32_t>); + static_assert( + std::same_as, std::uint32_t>); +} + +TEST_CASE("argument encoding", "[mipi]") { + static_assert( + std::same_as, encode_32>); + static_assert( + std::same_as, encode_u32>); + static_assert( + std::same_as, encode_64>); + static_assert( + std::same_as, encode_u64>); + static_assert(std::same_as, encode_32>); + static_assert( + std::same_as, 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{}}; + cfg.logger.log_msg(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; diff --git a/test/log/stable_strings.json b/test/log/stable_strings.json index 03fac264..32b7ea46 100644 --- a/test/log/stable_strings.json +++ b/test/log/stable_strings.json @@ -1,7 +1,6 @@ { "messages": [ { - "level": "TRACE", "msg": "An old string that can't be reused", "type": "msg", "arg_types": [], @@ -9,7 +8,6 @@ "id": 0 }, { - "level": "TRACE", "msg": "A string with no placeholders", "type": "msg", "arg_types": [], diff --git a/tools/gen_str_catalog.py b/tools/gen_str_catalog.py index 34bed6aa..9fd9bb27 100755 --- a/tools/gen_str_catalog.py +++ b/tools/gen_str_catalog.py @@ -255,6 +255,12 @@ def serialize_modules(client_node: et.Element, modules): syst_module.text = f"" +def arg_printf_spec(arg: str): + printf_dict = {"encode_32": "%d", "encode_u32": "%u", + "encode_64": "%lld", "encode_u64": "%llu"} + return printf_dict.get(arg, "%d") + + def serialize_messages(short_node: et.Element, catalog_node: et.Element, messages): for msg in messages.values(): syst_format = et.SubElement( @@ -267,9 +273,13 @@ def serialize_messages(short_node: et.Element, catalog_node: et.Element, message "EnumLookup", f'{msg["enum_lookup"][0]}:{msg["enum_lookup"][1]}' ) - fmt_string = re.sub(r"{}", r"%d", msg["msg"]) - if msg["arg_count"] != 0: - fmt_string = re.sub(r"{:(.*?)}", r"%\1", fmt_string) + fmt_string = msg["msg"] + for arg in msg["arg_types"]: + m = re.search(r"{:([^}]+)}", fmt_string) + if m: + fmt_string = f"{fmt_string[:m.start()]}%{m.group(1)}{fmt_string[m.end():]}" + else: + fmt_string = re.sub(r"{}", arg_printf_spec(arg), fmt_string, count = 1) syst_format.text = f""