Revert ":art: Autodetect scoped enums in generated .cpp catalog file"

elbeno · elbeno · commit c4780d1cea39 · 2025-10-06T15:29:59.000-06:00
This reverts commit a696f42.
diff --git a/include/log/catalog/arguments.hpp b/include/log/catalog/arguments.hpp
@@ -10,67 +10,48 @@ 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<T> and sizeof(T) <= sizeof(std::int64_t);
+concept signed_packable = std::signed_integral<stdx::underlying_type_t<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);
+    std::unsigned_integral<stdx::underlying_type_t<T>> and
+    sizeof(T) <= sizeof(std::int64_t);
 
 template <typename T>
-concept float_packable =
-    std::floating_point<T> and sizeof(T) <= sizeof(std::int64_t);
+concept float_packable = std::floating_point<stdx::underlying_type_t<T>> and
+                         sizeof(T) <= sizeof(std::int64_t);
 
 template <typename 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>;
+concept packable =
+    signed_packable<T> or unsigned_packable<T> or float_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 = detail::signed_encode_t<T>;
+    using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
+                                         encode_32<T>, encode_64<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 = detail::unsigned_encode_t<T>;
+    using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
+                                         encode_u32<T>, encode_u64<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 = detail::unsigned_encode_t<T>;
+    using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
+                                         encode_u32<T>, encode_u64<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,26 +26,14 @@ 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">(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));
+        stdx::ct_format<"E string with {} placeholder">(E::value));
 }
 
 auto log_rt_float_arg() -> void {
diff --git a/test/log/catalog_app.cpp b/test/log/catalog_app.cpp
@@ -17,7 +17,6 @@ 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;
@@ -101,15 +100,6 @@ 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 E { VAL = 42 };
-} // namespace ns
+enum struct E { value = 42 };
+}
diff --git a/test/log/encoder.cpp b/test/log/encoder.cpp
@@ -155,11 +155,6 @@ 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>);
@@ -170,8 +165,6 @@ 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]") {
@@ -189,10 +182,6 @@ 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,19 +202,6 @@ 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}"
@@ -234,16 +221,11 @@ def make_cpp_module_defn(m: Module) -> str:
 }}"""
 
 
-def write_cpp(messages, modules, scoped_enums, extra_headers: list[str], filename: str):
+def write_cpp(messages, modules, 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")
@@ -269,15 +251,18 @@ def fq_name(node):
     index = Index.create()
     translation_unit = index.parse(filename)
 
-    enums: dict[str, dict] = {}
+    enums = {}
     for node in translation_unit.cursor.walk_preorder():
         if node.kind == CursorKind.ENUM_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
+            enums.update(
+                {
+                    fq_name(node): {
+                        e.spelling: e.enum_value
+                        for e in node.walk_preorder()
+                        if e.kind == CursorKind.ENUM_CONSTANT_DECL
+                    }
+                }
+            )
     return enums
 
 
@@ -289,11 +274,11 @@ def write_json(
     )
     for m in stable_data.get("messages"):
         j = m.to_json()
-        if j not in d["messages"]:
+        if not j in d["messages"]:
             d["messages"].append(j)
     for m in stable_data.get("modules"):
         j = m.to_json()
-        if j not in d["modules"]:
+        if not j in d["modules"]:
             d["modules"].append(j)
 
     str_catalog = dict(**d, enums=dict())
@@ -362,13 +347,9 @@ 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|enum)<(.*)>")
+    string_re = re.compile(r"encode_(32|u32|64|u64)<(.*)>")
     m = string_re.match(arg)
-    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))
+    return (f"encode_{m.group(1)}", m.group(2))
 
 
 def arg_printf_spec(arg: str):
@@ -379,20 +360,9 @@ 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, _, ut = arg_type_encoding(arg)
-    return printf_dict.get(ut, printf_dict.get(enc, "%d"))
+    enc, t = arg_type_encoding(arg)
+    return printf_dict.get(t, printf_dict.get(enc, "%d"))
 
 
 def serialize_messages(
@@ -569,15 +539,8 @@ def main():
     except Exception as e:
         raise Exception(f"{str(e)} from file {args.input}")
 
-    scoped_enums = {}
     if args.cpp_output is not None:
-        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)
+        write_cpp(messages, modules, args.cpp_headers, args.cpp_output)
 
     stable_output = dict(messages=[], modules=[])
     if not args.forget_old_ids:
@@ -600,13 +563,14 @@ 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")
 
