Skip to content

Commit fcbe3df

Browse files
committed
🎨 Autodetect scoped enums in generated .cpp catalog file
Problem: - The `.cpp` file generated by `gen_str_catalog` requires headers to be supplied in order to define the enums used. If the headers are not supplied, it does not compile. Solution: - Export scoped enum information sufficient to forward declare enumerations. If headers are not supplied, the `.cpp` file should still compile. Note: - Unscoped enums cannot be forward declared. - Enums inside anonymous namespaces cannot be forward declared.
1 parent 3a6da7f commit fcbe3df

File tree

6 files changed

+123
-36
lines changed

6 files changed

+123
-36
lines changed

include/log/catalog/arguments.hpp

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,48 +10,66 @@ template <typename> struct encode_32;
1010
template <typename> struct encode_64;
1111
template <typename> struct encode_u32;
1212
template <typename> struct encode_u64;
13+
template <typename...> struct encode_enum;
1314

1415
namespace logging {
1516
template <typename T>
16-
concept signed_packable = std::signed_integral<stdx::underlying_type_t<T>> and
17-
sizeof(T) <= sizeof(std::int64_t);
17+
concept signed_packable =
18+
std::signed_integral<T> and sizeof(T) <= sizeof(std::int64_t);
1819

1920
template <typename T>
2021
concept unsigned_packable =
21-
std::unsigned_integral<stdx::underlying_type_t<T>> and
22-
sizeof(T) <= sizeof(std::int64_t);
22+
std::unsigned_integral<T> and sizeof(T) <= sizeof(std::int64_t);
2323

2424
template <typename T>
25-
concept float_packable = std::floating_point<stdx::underlying_type_t<T>> and
26-
sizeof(T) <= sizeof(std::int64_t);
25+
concept float_packable =
26+
std::floating_point<T> and sizeof(T) <= sizeof(std::int64_t);
2727

2828
template <typename T>
29-
concept packable =
30-
signed_packable<T> or unsigned_packable<T> or float_packable<T>;
29+
concept enum_packable = std::is_enum_v<T> and sizeof(T) <= sizeof(std::int64_t);
30+
31+
template <typename T>
32+
concept packable = signed_packable<T> or unsigned_packable<T> or
33+
float_packable<T> or enum_packable<T>;
3134

3235
template <typename T> struct encoding;
3336

37+
namespace detail {
38+
template <typename T>
39+
using signed_encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
40+
encode_32<T>, encode_64<T>>;
41+
template <typename T>
42+
using unsigned_encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
43+
encode_u32<T>, encode_u64<T>>;
44+
} // namespace detail
45+
3446
template <signed_packable T> struct encoding<T> {
35-
using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
36-
encode_32<T>, encode_64<T>>;
47+
using encode_t = detail::signed_encode_t<T>;
3748
using pack_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
3849
std::int32_t, std::int64_t>;
3950
};
4051

4152
template <unsigned_packable T> struct encoding<T> {
42-
using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
43-
encode_u32<T>, encode_u64<T>>;
53+
using encode_t = detail::unsigned_encode_t<T>;
4454
using pack_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
4555
std::uint32_t, std::uint64_t>;
4656
};
4757

4858
template <float_packable T> struct encoding<T> {
49-
using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
50-
encode_u32<T>, encode_u64<T>>;
59+
using encode_t = detail::unsigned_encode_t<T>;
5160
using pack_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
5261
std::uint32_t, std::uint64_t>;
5362
};
5463

64+
template <enum_packable T>
65+
struct encoding<T> : encoding<stdx::underlying_type_t<T>> {
66+
using encode_t = stdx::conditional_t<
67+
stdx::is_scoped_enum_v<T>, encode_enum<T, stdx::underlying_type_t<T>>,
68+
stdx::conditional_t<std::signed_integral<stdx::underlying_type_t<T>>,
69+
detail::signed_encode_t<T>,
70+
detail::unsigned_encode_t<T>>>;
71+
};
72+
5573
struct default_arg_packer {
5674
template <packable T> using pack_as_t = typename encoding<T>::pack_t;
5775
template <packable T> using encode_as_t = typename encoding<T>::encode_t;

test/log/catalog2b_lib.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,26 @@ using log_env2b = stdx::make_env_t<logging::get_level, logging::level::TRACE>;
2222
} // namespace
2323

2424
auto log_rt_enum_arg() -> void;
25+
auto log_rt_auto_scoped_enum_arg() -> void;
2526
auto log_rt_float_arg() -> void;
2627
auto log_rt_double_arg() -> void;
2728

2829
auto log_rt_enum_arg() -> void {
2930
auto cfg = logging::binary::config{test_log_args_destination{}};
3031
using namespace ns;
3132
cfg.logger.log_msg<log_env2b>(
32-
stdx::ct_format<"E string with {} placeholder">(E::value));
33+
stdx::ct_format<"E string with {} placeholder">(VAL));
34+
}
35+
36+
namespace some_ns {
37+
enum struct E { A, B, C };
38+
}
39+
40+
auto log_rt_auto_scoped_enum_arg() -> void {
41+
auto cfg = logging::binary::config{test_log_args_destination{}};
42+
using namespace some_ns;
43+
cfg.logger.log_msg<log_env2b>(
44+
stdx::ct_format<"E (scoped) string with {} placeholder">(E::A));
3345
}
3446

3547
auto log_rt_float_arg() -> void {

test/log/catalog_app.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ extern auto log_one_64bit_rt_arg() -> void;
1717
extern auto log_one_formatted_rt_arg() -> void;
1818
extern auto log_two_rt_args() -> void;
1919
extern auto log_rt_enum_arg() -> void;
20+
extern auto log_rt_auto_scoped_enum_arg() -> void;
2021
extern auto log_with_non_default_module() -> void;
2122
extern auto log_with_fixed_module() -> void;
2223
extern auto log_with_fixed_string_id() -> void;
@@ -100,6 +101,15 @@ TEST_CASE("log runtime enum argument", "[catalog]") {
100101
CHECK(log_calls == 1);
101102
}
102103

104+
TEST_CASE("log runtime scoped enum argument outside included header",
105+
"[catalog]") {
106+
log_calls = 0;
107+
test_critical_section::count = 0;
108+
log_rt_auto_scoped_enum_arg();
109+
CHECK(test_critical_section::count == 2);
110+
CHECK(log_calls == 1);
111+
}
112+
103113
TEST_CASE("log module ids change", "[catalog]") {
104114
// subtype 1, severity 7, type 3
105115
std::uint32_t expected_static = (1u << 24u) | (7u << 4u) | 3u;

test/log/catalog_enums.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#pragma once
22

33
namespace ns {
4-
enum struct E { value = 42 };
5-
}
4+
enum E { VAL = 42 };
5+
} // namespace ns

test/log/encoder.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,11 @@ using log_env = stdx::make_env_t<logging::get_level, logging::level::TRACE>;
153153

154154
template <> inline auto conc::injected_policy<> = test_conc_policy{};
155155

156+
namespace {
157+
enum UnscopedE { A, B, C };
158+
enum struct ScopedE { A, B, C };
159+
} // namespace
160+
156161
TEST_CASE("argument packing", "[mipi]") {
157162
using P = logging::default_arg_packer;
158163
STATIC_REQUIRE(std::same_as<P::pack_as_t<std::int32_t>, std::int32_t>);
@@ -163,6 +168,8 @@ TEST_CASE("argument packing", "[mipi]") {
163168
STATIC_REQUIRE(std::same_as<P::pack_as_t<unsigned char>, std::uint32_t>);
164169
STATIC_REQUIRE(std::same_as<P::pack_as_t<float>, std::uint32_t>);
165170
STATIC_REQUIRE(std::same_as<P::pack_as_t<double>, std::uint64_t>);
171+
STATIC_REQUIRE(std::same_as<P::pack_as_t<UnscopedE>, std::uint32_t>);
172+
STATIC_REQUIRE(std::same_as<P::pack_as_t<ScopedE>, std::int32_t>);
166173
}
167174

168175
TEST_CASE("argument encoding", "[mipi]") {
@@ -180,6 +187,10 @@ TEST_CASE("argument encoding", "[mipi]") {
180187
std::same_as<P::encode_as_t<unsigned char>, encode_u32<unsigned char>>);
181188
STATIC_REQUIRE(std::same_as<P::encode_as_t<float>, encode_u32<float>>);
182189
STATIC_REQUIRE(std::same_as<P::encode_as_t<double>, encode_u64<double>>);
190+
STATIC_REQUIRE(
191+
std::same_as<P::encode_as_t<UnscopedE>, encode_u32<UnscopedE>>);
192+
STATIC_REQUIRE(
193+
std::same_as<P::encode_as_t<ScopedE>, encode_enum<ScopedE, int>>);
183194
}
184195

185196
TEST_CASE("log zero arguments", "[mipi]") {

tools/gen_str_catalog.py

Lines changed: 55 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,19 @@ def read_file(filename):
202202
return assign_ids(unique_messages, unique_modules, stable_data)
203203

204204

205+
def make_cpp_scoped_enum_decl(e: str, ut: str) -> str:
206+
parts = e.split("::")
207+
enum = parts[-1]
208+
if "(anonymous namespace)" in parts or "{anonymous}" in parts:
209+
raise Exception(
210+
f"Scoped enum {e} is inside an anonymous namespace and cannot be forward declared."
211+
)
212+
if len(parts) > 1:
213+
ns = "::".join(parts[:-1])
214+
return f"namespace {ns} {{ enum struct {enum} : {ut}; }}"
215+
return f"enum struct {enum} : {ut};"
216+
217+
205218
def make_cpp_catalog_defn(m: Message) -> str:
206219
return f"""/*
207220
"{m.text}"
@@ -221,11 +234,16 @@ def make_cpp_module_defn(m: Module) -> str:
221234
}}"""
222235

223236

224-
def write_cpp(messages, modules, extra_headers: list[str], filename: str):
237+
def write_cpp(messages, modules, scoped_enums, extra_headers: list[str], filename: str):
225238
with open(filename, "w") as f:
226239
f.write("\n".join(f'#include "{h}"' for h in extra_headers))
227240
f.write("\n#include <log/catalog/arguments.hpp>\n")
228241
f.write("\n#include <log/catalog/catalog.hpp>\n\n")
242+
scoped_enum_decls = [
243+
make_cpp_scoped_enum_decl(e, ut) for e, ut in scoped_enums.items()
244+
]
245+
f.write("\n".join(scoped_enum_decls))
246+
f.write("\n\n")
229247
cpp_catalog_defns = [make_cpp_catalog_defn(m) for m in messages]
230248
f.write("\n".join(cpp_catalog_defns))
231249
f.write("\n\n")
@@ -254,15 +272,12 @@ def fq_name(node):
254272
enums = {}
255273
for node in translation_unit.cursor.walk_preorder():
256274
if node.kind == CursorKind.ENUM_DECL:
257-
enums.update(
258-
{
259-
fq_name(node): {
260-
e.spelling: e.enum_value
261-
for e in node.walk_preorder()
262-
if e.kind == CursorKind.ENUM_CONSTANT_DECL
263-
}
264-
}
265-
)
275+
new_decl = {
276+
e.spelling: e.enum_value
277+
for e in node.walk_preorder()
278+
if e.kind == CursorKind.ENUM_CONSTANT_DECL
279+
}
280+
enums[fq_name(node)] = enums.get(fq_name(node), {}) | new_decl
266281
return enums
267282

268283

@@ -274,11 +289,11 @@ def write_json(
274289
)
275290
for m in stable_data.get("messages"):
276291
j = m.to_json()
277-
if not j in d["messages"]:
292+
if j not in d["messages"]:
278293
d["messages"].append(j)
279294
for m in stable_data.get("modules"):
280295
j = m.to_json()
281-
if not j in d["modules"]:
296+
if j not in d["modules"]:
282297
d["modules"].append(j)
283298

284299
str_catalog = dict(**d, enums=dict())
@@ -347,9 +362,13 @@ def serialize_modules(client_node: et.Element, modules: list[Module]):
347362

348363

349364
def arg_type_encoding(arg):
350-
string_re = re.compile(r"encode_(32|u32|64|u64)<(.*)>")
365+
string_re = re.compile(r"encode_(32|u32|64|u64|enum)<(.*)>")
351366
m = string_re.match(arg)
352-
return (f"encode_{m.group(1)}", m.group(2))
367+
if "enum" in m.group(1):
368+
args_re = re.compile(r"(.*), (.*)")
369+
args_m = args_re.match(m.group(2))
370+
return (f"encode_{m.group(1)}", args_m.group(1), args_m.group(2))
371+
return (f"encode_{m.group(1)}", m.group(2), m.group(2))
353372

354373

355374
def arg_printf_spec(arg: str):
@@ -360,9 +379,20 @@ def arg_printf_spec(arg: str):
360379
"encode_u64": "%llu",
361380
"float": "%f",
362381
"double": "%f",
382+
"int": "%d",
383+
"unsigned int": "%u",
384+
"short": "%d",
385+
"unsigned short": "%u",
386+
"signed char": "%d",
387+
"unsigned char": "%u",
388+
"char": "%c",
389+
"long": "%ld",
390+
"unsigned long": "%lu",
391+
"long long": "%lld",
392+
"unsigned long long": "%llu",
363393
}
364-
enc, t = arg_type_encoding(arg)
365-
return printf_dict.get(t, printf_dict.get(enc, "%d"))
394+
enc, _, ut = arg_type_encoding(arg)
395+
return printf_dict.get(ut, printf_dict.get(enc, "%d"))
366396

367397

368398
def serialize_messages(
@@ -539,8 +569,15 @@ def main():
539569
except Exception as e:
540570
raise Exception(f"{str(e)} from file {args.input}")
541571

572+
scoped_enums = {}
542573
if args.cpp_output is not None:
543-
write_cpp(messages, modules, args.cpp_headers, args.cpp_output)
574+
for m in messages:
575+
for i, arg_type in enumerate(m.args):
576+
enc, enum, ut = arg_type_encoding(arg_type)
577+
if "enum" in enc:
578+
scoped_enums.update({enum: ut})
579+
580+
write_cpp(messages, modules, scoped_enums, args.cpp_headers, args.cpp_output)
544581

545582
stable_output = dict(messages=[], modules=[])
546583
if not args.forget_old_ids:
@@ -563,14 +600,13 @@ def main():
563600
}
564601
for m in messages:
565602
for i, arg_type in enumerate(m.args):
566-
_, enum = arg_type_encoding(arg_type)
603+
_, enum, _ = arg_type_encoding(arg_type)
567604
if enum in enums:
568605
m.enum_lookup = (i + 1, enums[enum][0])
569606
except Exception as e:
570607
print(
571608
f"Couldn't extract enum info ({str(e)}), enum lookup will not be available"
572609
)
573-
574610
else:
575611
print("XML output without C++ output: enum lookup will not be available")
576612

0 commit comments

Comments
 (0)