From c4780d1cea39ea70cdb5aad804ce8722b096d52a Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Mon, 6 Oct 2025 15:29:59 -0600 Subject: [PATCH] Revert ":art: Autodetect scoped enums in generated `.cpp` catalog file" This reverts commit a696f42c24cc4761198fb100277513bf32b28901. --- include/log/catalog/arguments.hpp | 47 ++++++------------- test/log/catalog2b_lib.cpp | 14 +----- test/log/catalog_app.cpp | 10 ---- test/log/catalog_enums.hpp | 4 +- test/log/encoder.cpp | 11 ----- tools/gen_str_catalog.py | 76 ++++++++----------------------- 6 files changed, 37 insertions(+), 125 deletions(-) diff --git a/include/log/catalog/arguments.hpp b/include/log/catalog/arguments.hpp index 87664840..eb1bf7ff 100644 --- a/include/log/catalog/arguments.hpp +++ b/include/log/catalog/arguments.hpp @@ -10,67 +10,48 @@ template struct encode_32; template struct encode_64; template struct encode_u32; template struct encode_u64; -template struct encode_enum; namespace logging { template -concept signed_packable = - std::signed_integral and sizeof(T) <= sizeof(std::int64_t); +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); + std::unsigned_integral> and + sizeof(T) <= sizeof(std::int64_t); template -concept float_packable = - std::floating_point and sizeof(T) <= sizeof(std::int64_t); +concept float_packable = std::floating_point> and + sizeof(T) <= sizeof(std::int64_t); template -concept enum_packable = std::is_enum_v and sizeof(T) <= sizeof(std::int64_t); - -template -concept packable = signed_packable or unsigned_packable or - float_packable or enum_packable; +concept packable = + signed_packable or unsigned_packable or float_packable; template struct encoding; -namespace detail { -template -using signed_encode_t = stdx::conditional_t, encode_64>; -template -using unsigned_encode_t = - stdx::conditional_t, - encode_u64>; -} // namespace detail - template struct encoding { - using encode_t = detail::signed_encode_t; + using encode_t = stdx::conditional_t, encode_64>; using pack_t = stdx::conditional_t; }; template struct encoding { - using encode_t = detail::unsigned_encode_t; + using encode_t = stdx::conditional_t, encode_u64>; using pack_t = stdx::conditional_t; }; template struct encoding { - using encode_t = detail::unsigned_encode_t; + using encode_t = stdx::conditional_t, encode_u64>; using pack_t = stdx::conditional_t; }; -template -struct encoding : encoding> { - using encode_t = stdx::conditional_t< - stdx::is_scoped_enum_v, encode_enum>, - stdx::conditional_t>, - detail::signed_encode_t, - detail::unsigned_encode_t>>; -}; - struct default_arg_packer { template using pack_as_t = typename encoding::pack_t; template using encode_as_t = typename encoding::encode_t; diff --git a/test/log/catalog2b_lib.cpp b/test/log/catalog2b_lib.cpp index e737d880..9f9de82c 100644 --- a/test/log/catalog2b_lib.cpp +++ b/test/log/catalog2b_lib.cpp @@ -26,7 +26,6 @@ using log_env2b = stdx::make_env_t; } // 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; @@ -34,18 +33,7 @@ auto log_rt_enum_arg() -> void { auto cfg = logging::binary::config{test_log_destination{}}; using namespace ns; cfg.logger.log_msg( - 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( - 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 index 19eaee3e..34690e89 100644 --- 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 index b6c8ed59..f378443b 100644 --- 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 index a1731e9a..70c1b99d 100644 --- a/test/log/encoder.cpp +++ b/test/log/encoder.cpp @@ -155,11 +155,6 @@ using log_env = stdx::make_env_t; 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, std::int32_t>); @@ -170,8 +165,6 @@ TEST_CASE("argument packing", "[mipi]") { STATIC_REQUIRE(std::same_as, std::uint32_t>); STATIC_REQUIRE(std::same_as, std::uint32_t>); STATIC_REQUIRE(std::same_as, std::uint64_t>); - STATIC_REQUIRE(std::same_as, std::uint32_t>); - STATIC_REQUIRE(std::same_as, std::int32_t>); } TEST_CASE("argument encoding", "[mipi]") { @@ -189,10 +182,6 @@ TEST_CASE("argument encoding", "[mipi]") { std::same_as, encode_u32>); STATIC_REQUIRE(std::same_as, encode_u32>); STATIC_REQUIRE(std::same_as, encode_u64>); - STATIC_REQUIRE( - std::same_as, encode_u32>); - STATIC_REQUIRE( - std::same_as, encode_enum>); } TEST_CASE("log zero arguments", "[mipi]") { diff --git a/tools/gen_str_catalog.py b/tools/gen_str_catalog.py index a470d06a..6661df4b 100755 --- 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 \n") f.write("\n#include \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")