🎨 Separate MIPI support from binary logger machinery

Problem:
- It is useful to have a binary message logger for any format, not just for
  MIPI-SyS-T.

Solution:
- Separate the MIPI message building entirely from the binary catalog-based
  logger.

Author: elbeno

CMakeLists.txt

@@ -193,13 +193,13 @@ target_sources(
               FILES
               include/log/fmt/logger.hpp)
 
-add_library(cib_log_mipi INTERFACE)
-target_compile_features(cib_log_mipi INTERFACE cxx_std_20)
-target_link_libraries_system(cib_log_mipi INTERFACE cib_log cib_msg concurrency
-                             stdx)
+add_library(cib_log_binary INTERFACE)
+target_compile_features(cib_log_binary INTERFACE cxx_std_20)
+target_link_libraries_system(cib_log_binary INTERFACE cib_log cib_msg
+                             concurrency stdx)
 
 target_sources(
-    cib_log_mipi
+    cib_log_binary
     INTERFACE FILE_SET
               log
               TYPE
@@ -208,8 +208,8 @@ target_sources(
               include
               FILES
               include/log/catalog/catalog.hpp
+              include/log/catalog/encoder.hpp
               include/log/catalog/mipi_builder.hpp
-              include/log/catalog/mipi_encoder.hpp
               include/log/catalog/mipi_messages.hpp)
 
 add_library(cib_nexus INTERFACE)
@@ -298,8 +298,8 @@ target_link_libraries_system(
     cib_flow
     cib_interrupt
     cib_log
+    cib_log_binary
     cib_log_fmt
-    cib_log_mipi
     cib_lookup
     cib_match
     cib_msg
@@ -329,8 +329,8 @@ if(PROJECT_IS_TOP_LEVEL)
     clang_tidy_interface(cib_interrupt)
     clang_tidy_interface(cib_lookup)
     clang_tidy_interface(cib_log)
+    clang_tidy_interface(cib_log_binary)
     clang_tidy_interface(cib_log_fmt)
-    clang_tidy_interface(cib_log_mipi)
     clang_tidy_interface(cib_match)
     clang_tidy_interface(cib_msg)
     clang_tidy_interface(cib_nexus)

docs/intro.adoc

@@ -73,8 +73,8 @@ flowchart BT
   msg(cib_msg) --> log & match
   msg --> lookup
 
-  log_mipi(cib_log_mipi) --> msg
+  log_binary(cib_log_binary) --> msg
   seq(cib_seq) --> flow
 
-  cib --> interrupt & seq & log_fmt & log_mipi
+  cib --> interrupt & seq & log_fmt & log_binary
 ----

docs/logging.adoc

@@ -14,7 +14,7 @@ Logging in _cib_ is in two parts:
 Three possible logger implementations are provided:
 
 - one using fmt in https://github.com/intel/compile-time-init-build/tree/main/include/log/fmt/logger.hpp[fmt/logger.hpp]
-- one using the https://www.mipi.org/specifications/sys-t[MIPI Sys-T spec] in https://github.com/intel/compile-time-init-build/tree/main/include/log/catalog/mipi_encoder.hpp[catalog/mipi_encoder.hpp]
+- one using binary encoding in https://github.com/intel/compile-time-init-build/tree/main/include/log/catalog/encoder.hpp[catalog/encoder.hpp], using the https://www.mipi.org/specifications/sys-t[MIPI Sys-T spec] by default
 - the default implementation: the null logger which accepts everything, but never produces output
 
 === Log levels
@@ -208,22 +208,22 @@ template <> inline auto version::config<> = my_version_config{};
 ----
 
 Then use `CIB_LOG_VERSION()` to log the version. If the logging config provides
-a `log_build` function, that will be used. Otherwise a text string will be
+a `log_version` function, that will be used. Otherwise a text string will be
 logged.
 
 [source,cpp]
 ----
 struct my_logger_config {
   struct {
-    template <auto Version, stdx::ct_string S = ""> auto log_build() -> void {
+    template <auto Version, stdx::ct_string S = ""> auto log_version() -> void {
       // log the build version according to my mechanism
     }
   } logger;
 };
 template <>
 inline auto logging::config<> = my_logger_config{};
 
-CIB_LOG_VERSION(); // calls my_logger_config::log_build
+CIB_LOG_VERSION(); // calls my_logger_config::log_version
 ----
 
 The easiest way to flavor the version logging is to define a macro in terms of

docs/sc.adoc

@@ -43,7 +43,7 @@ static_assert("Hello,"_sc + " World!"_sc == "Hello, World!"_sc);
 The reason `string_constant` exists is for efficient logging. On a constrained
 system, space for text can be limited-to-nonexistent. `string_constant`
 interacts with the
-https://github.com/intel/compile-time-init-build/tree/main/include/log/catalog/mipi_encoder.hpp[MIPI
+https://github.com/intel/compile-time-init-build/tree/main/include/log/catalog/encoder.hpp[MIPI
 Sys-T logging config] to solve this problem.
 
 - First, each `string_constant` contains string character data in its type.

include/log/README.md

@@ -6,7 +6,7 @@ Logging in *cib* is in two parts:
 
 Three possible logger implementations are provided:
 - one using libfmt in [fmt/logger.hpp](fmt/logger.hpp)
-- one using the [MIPI SyS-T spec](https://www.mipi.org/specifications/sys-t), in [catalog/mipi_encoder.hpp](catalog/mipi_encoder.hpp)
+- a binary logger in [catalog/encoder.hpp](catalog/encoder.hpp), by default using the [MIPI SyS-T spec](https://www.mipi.org/specifications/sys-t)
 - the null logger (accepts everything, never produces output)
 
 ## log levels

include/log/catalog/builder.hpp

@@ -0,0 +1,21 @@
+#include <log/catalog/mipi_builder.hpp>
+
+#include <stdx/compiler.hpp>
+
+#include <utility>
+
+namespace logging::binary {
+[[maybe_unused]] constexpr inline struct get_builder_t {
+    template <typename T>
+        requires true
+    CONSTEVAL auto operator()(T &&t) const noexcept(
+        noexcept(std::forward<T>(t).query(std::declval<get_builder_t>())))
+        -> decltype(std::forward<T>(t).query(*this)) {
+        return std::forward<T>(t).query(*this);
+    }
+
+    CONSTEVAL auto operator()(auto &&) const {
+        return logging::mipi::default_builder{};
+    }
+} get_builder;
+} // namespace logging::binary

include/log/catalog/mipi_encoder.hpp renamed to include/log/catalog/encoder.hpp

@@ -1,42 +1,44 @@
 #pragma once
 
+#include <log/catalog/builder.hpp>
 #include <log/catalog/catalog.hpp>
-#include <log/catalog/mipi_builder.hpp>
-#include <log/catalog/mipi_messages.hpp>
 #include <log/log.hpp>
 #include <log/module.hpp>
 
-#include <stdx/bit.hpp>
-#include <stdx/compiler.hpp>
 #include <stdx/ct_string.hpp>
+#include <stdx/span.hpp>
 #include <stdx/tuple.hpp>
 #include <stdx/utility.hpp>
 
 #include <conc/concurrency.hpp>
 
-#include <algorithm>
-#include <concepts>
+#include <cstddef>
 #include <cstdint>
 #include <string_view>
+#include <type_traits>
 #include <utility>
 
-namespace logging::mipi {
+namespace logging::binary {
 namespace detail {
-template <typename S, typename... Args> constexpr auto to_message() {
+template <typename S, typename... Args> constexpr static auto to_message() {
     constexpr auto s = S::value;
     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<encode_as_t<Args>...>, char_t, s[Is]...>>{};
+            sc::undefined<sc::args<Args...>, char_t, s[Is]...>>{};
     }(std::make_integer_sequence<std::size_t, std::size(s)>{});
 }
 
-template <stdx::ct_string S> constexpr auto to_module() {
+template <stdx::ct_string S> constexpr static auto to_module() {
     constexpr auto s = std::string_view{S};
     return [&]<std::size_t... Is>(std::integer_sequence<std::size_t, Is...>) {
         return sc::module_string<sc::undefined<void, char, s[Is]...>>{};
     }(std::make_integer_sequence<std::size_t, std::size(s)>{});
 }
+
+template <typename S> struct to_message_t {
+    template <typename... Args> using fn = decltype(to_message<S, Args...>());
+};
 } // namespace detail
 
 template <typename Destinations> struct log_writer {
@@ -62,6 +64,10 @@ template <typename Destinations> struct log_writer {
         }(std::make_index_sequence<N>{});
     }
 
+    auto operator()(auto const &msg) -> void {
+        this->operator()(msg.as_const_view().data());
+    }
+
     Destinations dests;
 };
 template <typename T> log_writer(T) -> log_writer<T>;
@@ -75,46 +81,22 @@ template <typename Writer> struct log_handler {
 
     template <typename Env, typename Msg> auto log_msg(Msg msg) -> void {
         msg.apply([&]<typename S, typename... Args>(S, Args... args) {
+            auto builder = get_builder(Env{});
             constexpr auto L = stdx::to_underlying(get_level(Env{}));
-            using Message = decltype(detail::to_message<S, Args...>());
+            using Message = decltype(builder)::template convert_args<
+                detail::to_message_t<S>::template fn, Args...>;
             using Module = decltype(detail::to_module<get_module(Env{})>());
-            auto builder = get_builder(Env{});
-            write(builder.template build<L>(catalog<Message>(),
-                                            module<Module>(), args...));
+            w(builder.template build<L>(catalog<Message>(), module<Module>(),
+                                        args...));
         });
     }
 
-    template <auto Version, stdx::ct_string S = ""> auto log_build() -> void {
-        using namespace msg;
-        if constexpr (S.empty() and stdx::bit_width(Version) <= 22) {
-            owning<defn::compact32_build_msg_t> message{"build_id"_field =
-                                                            Version};
-            write(message);
-        } else if constexpr (S.empty() and stdx::bit_width(Version) <= 54) {
-            owning<defn::compact64_build_msg_t> message{"build_id"_field =
-                                                            Version};
-            write(message);
-        } else {
-            constexpr auto header_size =
-                defn::normal_build_msg_t::size<std::uint8_t>::value;
-            constexpr auto payload_len = S.size() + sizeof(std::uint64_t);
-            using storage_t =
-                std::array<std::uint8_t, header_size + payload_len>;
-
-            defn::normal_build_msg_t::owner_t<storage_t> message{
-                "payload_len"_field = payload_len};
-            auto dest = &message.data()[header_size];
-
-            auto const ver = stdx::to_le(static_cast<std::uint64_t>(Version));
-            dest = std::copy_n(stdx::bit_cast<std::uint8_t const *>(&ver),
-                               sizeof(std::uint64_t), dest);
-            std::copy_n(std::cbegin(S.value), S.size(), dest);
-            write(message);
-        }
+    template <typename Env, auto Version, stdx::ct_string S = "">
+    auto log_version() -> void {
+        auto builder = get_builder(Env{});
+        w(builder.template build_version<Version, S>());
     }
 
-    auto write(auto const &msg) -> void { w(msg.as_const_view().data()); }
-
     Writer w;
 };
 
@@ -126,4 +108,4 @@ template <typename... TDestinations> struct config {
     log_handler<log_writer<destinations_tuple_t>> logger;
 };
 template <typename... Ts> config(Ts...) -> config<Ts...>;
-} // namespace logging::mipi
+} // namespace logging::binary

include/log/catalog/mipi_builder.hpp

@@ -9,6 +9,7 @@
 
 #include <array>
 #include <concepts>
+#include <cstddef>
 #include <cstdint>
 #include <cstring>
 #include <utility>
@@ -111,6 +112,40 @@ template <> struct builder<defn::catalog_msg_t> {
     }
 };
 
+template <> struct builder<defn::compact32_build_msg_t> {
+    template <auto Version> static auto build() {
+        using namespace msg;
+        return owning<defn::compact32_build_msg_t>{"build_id"_field = Version};
+    }
+};
+
+template <> struct builder<defn::compact64_build_msg_t> {
+    template <auto Version> static auto build() {
+        using namespace msg;
+        return owning<defn::compact64_build_msg_t>{"build_id"_field = Version};
+    }
+};
+
+template <> struct builder<defn::normal_build_msg_t> {
+    template <auto Version, stdx::ct_string S> static auto build() {
+        using namespace msg;
+        constexpr auto header_size =
+            defn::normal_build_msg_t::size<std::uint8_t>::value;
+        constexpr auto payload_len = S.size() + sizeof(std::uint64_t);
+        using storage_t = std::array<std::uint8_t, header_size + payload_len>;
+
+        defn::normal_build_msg_t::owner_t<storage_t> message{
+            "payload_len"_field = payload_len};
+        auto dest = &message.data()[header_size];
+
+        auto const ver = stdx::to_le(static_cast<std::uint64_t>(Version));
+        dest = std::copy_n(stdx::bit_cast<std::uint8_t const *>(&ver),
+                           sizeof(std::uint64_t), dest);
+        std::copy_n(std::cbegin(S.value), S.size(), dest);
+        return message;
+    }
+};
+
 struct default_builder {
     template <auto Level, packable... Ts>
     static auto build(string_id id, module_id m, Ts... args) {
@@ -121,17 +156,22 @@ struct default_builder {
                 id, m, args...);
         }
     }
-};
 
-[[maybe_unused]] constexpr inline struct get_builder_t {
-    template <typename T>
-        requires true
-    CONSTEVAL auto operator()(T &&t) const noexcept(
-        noexcept(std::forward<T>(t).query(std::declval<get_builder_t>())))
-        -> decltype(std::forward<T>(t).query(*this)) {
-        return std::forward<T>(t).query(*this);
+    template <auto Version, stdx::ct_string S = ""> auto build_version() {
+        using namespace msg;
+        if constexpr (S.empty() and stdx::bit_width(Version) <= 22) {
+            return builder<defn::compact32_build_msg_t>{}
+                .template build<Version>();
+        } else if constexpr (S.empty() and stdx::bit_width(Version) <= 54) {
+            return builder<defn::compact64_build_msg_t>{}
+                .template build<Version>();
+        } else {
+            return builder<defn::normal_build_msg_t>{}
+                .template build<Version, S>();
+        }
     }
 
-    CONSTEVAL auto operator()(auto &&) const { return default_builder{}; }
-} get_builder;
+    template <template <typename...> typename F, typename... Args>
+    using convert_args = F<encode_as_t<Args>...>;
+};
 } // namespace logging::mipi

include/log/log.hpp

@@ -93,10 +93,11 @@ template <typename Env, typename... Ts> static auto log_version() -> void {
     auto &l_cfg = get_config<Env, Ts...>();
     auto &v_cfg = ::version::config<Ts...>;
     if constexpr (requires {
-                      l_cfg.logger.template log_build<v_cfg.build_id,
-                                                      v_cfg.version_string>();
+                      l_cfg.logger.template log_version<Env, v_cfg.build_id,
+                                                        v_cfg.version_string>();
                   }) {
-        l_cfg.logger.template log_build<v_cfg.build_id, v_cfg.version_string>();
+        l_cfg.logger
+            .template log_version<Env, v_cfg.build_id, v_cfg.version_string>();
     } else {
         CIB_LOG_ENV(logging::get_level, logging::level::MAX);
         l_cfg.logger.template log<cib_log_env_t>(

test/log/CMakeLists.txt

@@ -7,7 +7,7 @@ add_tests(
     LIBRARIES
     cib_log)
 add_tests(FILES fmt_logger LIBRARIES cib_log_fmt)
-add_tests(FILES mipi_encoder mipi_logger LIBRARIES cib_log_mipi)
+add_tests(FILES encoder mipi_logger LIBRARIES cib_log_binary)
 
 add_library(catalog1_lib STATIC catalog1_lib.cpp)
 add_library(catalog2_lib OBJECT catalog2a_lib.cpp catalog2b_lib.cpp)
@@ -50,7 +50,7 @@ add_unit_test(
     catalog_app.cpp
     LIBRARIES
     warnings
-    cib_log_mipi
+    cib_log_binary
     catalog1_lib
     catalog2_lib
     catalog_strings)