Merge pull request #751 from elbeno/auto-enum-log

lukevalenty · web-flow · commit a4bffd849005 · 2025-10-06T14:24:39.000-07:00
🎨 Autodetect scoped enums in generated `.cpp` catalog file
diff --git a/include/log/catalog/arguments.hpp b/include/log/catalog/arguments.hpp
@@ -10,48 +10,67 @@ template <typename> struct encode_32;
 template <typename> struct encode_64;
 template <typename> struct encode_u32;
 template <typename> struct encode_u64;
+template <typename...> struct encode_enum;
 
 namespace logging {
 template <typename T>
-concept signed_packable = std::signed_integral<stdx::underlying_type_t<T>> and
-                          sizeof(T) <= sizeof(std::int64_t);
+concept signed_packable =
+    std::signed_integral<T> and sizeof(T) <= sizeof(std::int64_t);
 
 template <typename T>
 concept unsigned_packable =
-    std::unsigned_integral<stdx::underlying_type_t<T>> and
-    sizeof(T) <= sizeof(std::int64_t);
+    std::unsigned_integral<T> and sizeof(T) <= sizeof(std::int64_t);
 
 template <typename T>
-concept float_packable = std::floating_point<stdx::underlying_type_t<T>> and
-                         sizeof(T) <= sizeof(std::int64_t);
+concept float_packable =
+    std::floating_point<T> and sizeof(T) <= sizeof(std::int64_t);
 
 template <typename T>
-concept packable =
-    signed_packable<T> or unsigned_packable<T> or float_packable<T>;
+concept enum_packable = std::is_enum_v<T> and sizeof(T) <= sizeof(std::int64_t);
+
+template <typename T>
+concept packable = signed_packable<T> or unsigned_packable<T> or
+                   float_packable<T> or enum_packable<T>;
 
 template <typename T> struct encoding;
 
+namespace detail {
+template <typename T>
+using signed_encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
+                                            encode_32<T>, encode_64<T>>;
+template <typename T>
+using unsigned_encode_t =
+    stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t), encode_u32<T>,
+                        encode_u64<T>>;
+} // namespace detail
+
 template <signed_packable T> struct encoding<T> {
-    using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
-                                         encode_32<T>, encode_64<T>>;
+    using encode_t = detail::signed_encode_t<T>;
     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::uint32_t),
-                                         encode_u32<T>, encode_u64<T>>;
+    using encode_t = detail::unsigned_encode_t<T>;
     using pack_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
                                        std::uint32_t, std::uint64_t>;
 };
 
 template <float_packable T> struct encoding<T> {
-    using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
-                                         encode_u32<T>, encode_u64<T>>;
+    using encode_t = detail::unsigned_encode_t<T>;
     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>> {
+    using encode_t = stdx::conditional_t<
+        stdx::is_scoped_enum_v<T>, encode_enum<T, stdx::underlying_type_t<T>>,
+        stdx::conditional_t<std::signed_integral<stdx::underlying_type_t<T>>,
+                            detail::signed_encode_t<T>,
+                            detail::unsigned_encode_t<T>>>;
+};
+
 struct default_arg_packer {
     template <packable T> using pack_as_t = typename encoding<T>::pack_t;
     template <packable T> using encode_as_t = typename encoding<T>::encode_t;
diff --git a/test/log/catalog2b_lib.cpp b/test/log/catalog2b_lib.cpp
@@ -26,14 +26,26 @@ using log_env2b = stdx::make_env_t<logging::get_level, logging::level::TRACE>;
 } // namespace
 
 auto log_rt_enum_arg() -> void;
+auto log_rt_auto_scoped_enum_arg() -> void;
 auto log_rt_float_arg() -> void;
 auto log_rt_double_arg() -> void;
 
 auto log_rt_enum_arg() -> void {
     auto cfg = logging::binary::config{test_log_destination{}};
     using namespace ns;
     cfg.logger.log_msg<log_env2b>(
-        stdx::ct_format<"E string with {} placeholder">(E::value));
+        stdx::ct_format<"E string with {} placeholder">(VAL));
+}
+
+namespace some_ns {
+enum struct E { A, B, C };
+}
+
+auto log_rt_auto_scoped_enum_arg() -> void {
+    auto cfg = logging::binary::config{test_log_destination{}};
+    using namespace some_ns;
+    cfg.logger.log_msg<log_env2b>(
+        stdx::ct_format<"E (scoped) string with {} placeholder">(E::A));
 }
 
 auto log_rt_float_arg() -> void {
diff --git a/test/log/catalog_app.cpp b/test/log/catalog_app.cpp
@@ -17,6 +17,7 @@ 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_rt_auto_scoped_enum_arg() -> void;
 extern auto log_with_non_default_module() -> void;
 extern auto log_with_fixed_module() -> void;
 extern auto log_with_fixed_string_id() -> void;
@@ -100,6 +101,15 @@ TEST_CASE("log runtime enum argument", "[catalog]") {
     CHECK(log_calls == 1);
 }
 
+TEST_CASE("log runtime scoped enum argument outside included header",
+          "[catalog]") {
+    log_calls = 0;
+    test_critical_section::count = 0;
+    log_rt_auto_scoped_enum_arg();
+    CHECK(test_critical_section::count == 2);
+    CHECK(log_calls == 1);
+}
+
 TEST_CASE("log module ids change", "[catalog]") {
     // subtype 1, severity 7, type 3
     std::uint32_t expected_static = (1u << 24u) | (7u << 4u) | 3u;
diff --git a/test/log/catalog_enums.hpp b/test/log/catalog_enums.hpp
@@ -1,5 +1,5 @@
 #pragma once
 
 namespace ns {
-enum struct E { value = 42 };
-}
+enum E { VAL = 42 };
+} // namespace ns
diff --git a/test/log/encoder.cpp b/test/log/encoder.cpp
@@ -155,6 +155,11 @@ using log_env = stdx::make_env_t<logging::get_level, logging::level::TRACE>;
 
 template <> inline auto conc::injected_policy<> = test_conc_policy{};
 
+namespace {
+enum UnscopedE { A, B, C };
+enum struct ScopedE { A, B, C };
+} // namespace
+
 TEST_CASE("argument packing", "[mipi]") {
     using P = logging::default_arg_packer;
     STATIC_REQUIRE(std::same_as<P::pack_as_t<std::int32_t>, std::int32_t>);
@@ -165,6 +170,8 @@ TEST_CASE("argument packing", "[mipi]") {
     STATIC_REQUIRE(std::same_as<P::pack_as_t<unsigned char>, std::uint32_t>);
     STATIC_REQUIRE(std::same_as<P::pack_as_t<float>, std::uint32_t>);
     STATIC_REQUIRE(std::same_as<P::pack_as_t<double>, std::uint64_t>);
+    STATIC_REQUIRE(std::same_as<P::pack_as_t<UnscopedE>, std::uint32_t>);
+    STATIC_REQUIRE(std::same_as<P::pack_as_t<ScopedE>, std::int32_t>);
 }
 
 TEST_CASE("argument encoding", "[mipi]") {
@@ -182,6 +189,10 @@ TEST_CASE("argument encoding", "[mipi]") {
         std::same_as<P::encode_as_t<unsigned char>, encode_u32<unsigned char>>);
     STATIC_REQUIRE(std::same_as<P::encode_as_t<float>, encode_u32<float>>);
     STATIC_REQUIRE(std::same_as<P::encode_as_t<double>, encode_u64<double>>);
+    STATIC_REQUIRE(
+        std::same_as<P::encode_as_t<UnscopedE>, encode_u32<UnscopedE>>);
+    STATIC_REQUIRE(
+        std::same_as<P::encode_as_t<ScopedE>, encode_enum<ScopedE, int>>);
 }
 
 TEST_CASE("log zero arguments", "[mipi]") {
diff --git a/tools/gen_str_catalog.py b/tools/gen_str_catalog.py
@@ -202,6 +202,19 @@ def read_file(filename):
     return assign_ids(unique_messages, unique_modules, stable_data)
 
 
+def make_cpp_scoped_enum_decl(e: str, ut: str) -> str:
+    parts = e.split("::")
+    enum = parts[-1]
+    if "(anonymous namespace)" in parts or "{anonymous}" in parts:
+        raise Exception(
+            f"Scoped enum {e} is inside an anonymous namespace and cannot be forward declared."
+        )
+    if len(parts) > 1:
+        ns = "::".join(parts[:-1])
+        return f"namespace {ns} {{ enum struct {enum} : {ut}; }}"
+    return f"enum struct {enum} : {ut};"
+
+
 def make_cpp_catalog_defn(m: Message) -> str:
     return f"""/*
     "{m.text}"
@@ -221,11 +234,16 @@ def make_cpp_module_defn(m: Module) -> str:
 }}"""
 
 
-def write_cpp(messages, modules, extra_headers: list[str], filename: str):
+def write_cpp(messages, modules, scoped_enums, extra_headers: list[str], filename: str):
     with open(filename, "w") as f:
         f.write("\n".join(f'#include "{h}"' for h in extra_headers))
         f.write("\n#include <log/catalog/arguments.hpp>\n")
         f.write("\n#include <log/catalog/catalog.hpp>\n\n")
+        scoped_enum_decls = [
+            make_cpp_scoped_enum_decl(e, ut) for e, ut in scoped_enums.items()
+        ]
+        f.write("\n".join(scoped_enum_decls))
+        f.write("\n\n")
         cpp_catalog_defns = [make_cpp_catalog_defn(m) for m in messages]
         f.write("\n".join(cpp_catalog_defns))
         f.write("\n\n")
@@ -251,18 +269,15 @@ def fq_name(node):
     index = Index.create()
     translation_unit = index.parse(filename)
 
-    enums = {}
+    enums: dict[str, dict] = {}
     for node in translation_unit.cursor.walk_preorder():
         if node.kind == CursorKind.ENUM_DECL:
-            enums.update(
-                {
-                    fq_name(node): {
-                        e.spelling: e.enum_value
-                        for e in node.walk_preorder()
-                        if e.kind == CursorKind.ENUM_CONSTANT_DECL
-                    }
-                }
-            )
+            new_decl = {
+                e.spelling: e.enum_value
+                for e in node.walk_preorder()
+                if e.kind == CursorKind.ENUM_CONSTANT_DECL
+            }
+            enums[fq_name(node)] = enums.get(fq_name(node), {}) | new_decl
     return enums
 
 
@@ -274,11 +289,11 @@ def write_json(
     )
     for m in stable_data.get("messages"):
         j = m.to_json()
-        if not j in d["messages"]:
+        if j not in d["messages"]:
             d["messages"].append(j)
     for m in stable_data.get("modules"):
         j = m.to_json()
-        if not j in d["modules"]:
+        if j not in d["modules"]:
             d["modules"].append(j)
 
     str_catalog = dict(**d, enums=dict())
@@ -347,9 +362,13 @@ def serialize_modules(client_node: et.Element, modules: list[Module]):
 
 
 def arg_type_encoding(arg):
-    string_re = re.compile(r"encode_(32|u32|64|u64)<(.*)>")
+    string_re = re.compile(r"encode_(32|u32|64|u64|enum)<(.*)>")
     m = string_re.match(arg)
-    return (f"encode_{m.group(1)}", m.group(2))
+    if "enum" in m.group(1):
+        args_re = re.compile(r"(.*), (.*)")
+        args_m = args_re.match(m.group(2))
+        return (f"encode_{m.group(1)}", args_m.group(1), args_m.group(2))
+    return (f"encode_{m.group(1)}", m.group(2), m.group(2))
 
 
 def arg_printf_spec(arg: str):
@@ -360,9 +379,20 @@ def arg_printf_spec(arg: str):
         "encode_u64": "%llu",
         "float": "%f",
         "double": "%f",
+        "int": "%d",
+        "unsigned int": "%u",
+        "short": "%d",
+        "unsigned short": "%u",
+        "signed char": "%d",
+        "unsigned char": "%u",
+        "char": "%c",
+        "long": "%ld",
+        "unsigned long": "%lu",
+        "long long": "%lld",
+        "unsigned long long": "%llu",
     }
-    enc, t = arg_type_encoding(arg)
-    return printf_dict.get(t, printf_dict.get(enc, "%d"))
+    enc, _, ut = arg_type_encoding(arg)
+    return printf_dict.get(ut, printf_dict.get(enc, "%d"))
 
 
 def serialize_messages(
@@ -539,8 +569,15 @@ def main():
     except Exception as e:
         raise Exception(f"{str(e)} from file {args.input}")
 
+    scoped_enums = {}
     if args.cpp_output is not None:
-        write_cpp(messages, modules, args.cpp_headers, args.cpp_output)
+        for m in messages:
+            for i, arg_type in enumerate(m.args):
+                enc, enum, ut = arg_type_encoding(arg_type)
+                if "enum" in enc:
+                    scoped_enums.update({enum: ut})
+
+        write_cpp(messages, modules, scoped_enums, args.cpp_headers, args.cpp_output)
 
     stable_output = dict(messages=[], modules=[])
     if not args.forget_old_ids:
@@ -563,14 +600,13 @@ def main():
                 }
                 for m in messages:
                     for i, arg_type in enumerate(m.args):
-                        _, enum = arg_type_encoding(arg_type)
+                        _, enum, _ = arg_type_encoding(arg_type)
                         if enum in enums:
                             m.enum_lookup = (i + 1, enums[enum][0])
             except Exception as e:
                 print(
                     f"Couldn't extract enum info ({str(e)}), enum lookup will not be available"
                 )
-
         else:
             print("XML output without C++ output: enum lookup will not be available")
 
