diff --git a/include/log/catalog/arguments.hpp b/include/log/catalog/arguments.hpp index eb1bf7ff..87664840 100644 --- a/include/log/catalog/arguments.hpp +++ b/include/log/catalog/arguments.hpp @@ -10,48 +10,67 @@ 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 packable = - signed_packable or unsigned_packable or float_packable; +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; 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 = stdx::conditional_t, encode_64>; + using encode_t = detail::signed_encode_t; using pack_t = stdx::conditional_t; }; template struct encoding { - using encode_t = stdx::conditional_t, encode_u64>; + using encode_t = detail::unsigned_encode_t; using pack_t = stdx::conditional_t; }; template struct encoding { - using encode_t = stdx::conditional_t, encode_u64>; + using encode_t = detail::unsigned_encode_t; 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 9f9de82c..e737d880 100644 --- a/test/log/catalog2b_lib.cpp +++ b/test/log/catalog2b_lib.cpp @@ -26,6 +26,7 @@ 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; @@ -33,7 +34,18 @@ 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">(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( + 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 index 34690e89..19eaee3e 100644 --- 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 index f378443b..b6c8ed59 100644 --- 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 index 70c1b99d..a1731e9a 100644 --- a/test/log/encoder.cpp +++ b/test/log/encoder.cpp @@ -155,6 +155,11 @@ 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>); @@ -165,6 +170,8 @@ 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]") { @@ -182,6 +189,10 @@ 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 6661df4b..a470d06a 100755 --- 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 \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") @@ -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")