Skip to content

Commit a4723fb

Browse files
committed
tbv2: update std::variant
`std::variant` is the last archetypal container missing in TreeBuilder-v2. The code for it isn't hugely complicated and relies on pack expansion. This change introduces a new field to the container specification: `scoped_extra`. This field allows you to write extra code that will be included within the TypeHandler in CodeGen. This means it will not have collisions with other containers, unlike the existing `extra` field. It's used here to write the recursive `getSizeType` function for `std::variant`. Tech debt is introduced here by comparing the container name to `std::variant` in CodeGen to conditionally generate some code. We've worked hard to remove references to containers in code and move them to `.toml` files. On balance, this is worth having to include the example of `std::variant`. It should be moved into a container spec field at some point, the design of which is still to be determined. Test plan: - Activated the OIL `std::variant` tests. - CI
1 parent 7e18c4c commit a4723fb

File tree

6 files changed

+187
-39
lines changed

6 files changed

+187
-39
lines changed

oi/CodeGen.cpp

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -941,29 +941,45 @@ void genContainerTypeHandler(std::unordered_set<const ContainerInfo*>& used,
941941
containerWithTypes = "OICaptureKeys<" + containerWithTypes + ">";
942942
}
943943

944-
code += "template <typename Ctx";
945-
types = 0, values = 0;
946-
for (const auto& p : templateParams) {
947-
if (p.value) {
948-
code += ", ";
944+
// TODO: This is tech debt. This should be moved to a field in the container
945+
// spec/`.toml` called something like `codegen.handler_header` or have an
946+
// explicit option for variable template parameters. However I'm landing it
947+
// anyway to demonstrate how to handle tagged unions in TreeBuilder-v2.
948+
if (c.typeName == "std::variant") {
949+
code += R"(
950+
template <typename Ctx, typename... Types>
951+
struct TypeHandler<Ctx, std::variant<Types...>> {
952+
using container_type = std::variant<Types...>;
953+
)";
954+
} else {
955+
code += "template <typename Ctx";
956+
types = 0, values = 0;
957+
for (const auto& p : templateParams) {
958+
if (p.value) {
959+
code += ", ";
949960

950-
// HACK: forward all enums directly. this might turn out to be a problem
951-
// if there are enums we should be regenerating/use in the body.
952-
if (const auto* e = dynamic_cast<const Enum*>(&p.type())) {
953-
code += e->inputName();
961+
// HACK: forward all enums directly. this might turn out to be a problem
962+
// if there are enums we should be regenerating/use in the body.
963+
if (const auto* e = dynamic_cast<const Enum*>(&p.type())) {
964+
code += e->inputName();
965+
} else {
966+
code += p.type().name();
967+
}
968+
969+
code += " N" + std::to_string(values++);
954970
} else {
955-
code += p.type().name();
971+
code += ", typename T" + std::to_string(types++);
956972
}
957-
958-
code += " N" + std::to_string(values++);
959-
} else {
960-
code += ", typename T" + std::to_string(types++);
961973
}
974+
code += ">\n";
975+
code += "struct TypeHandler<Ctx, ";
976+
code += containerWithTypes;
977+
code += "> {\n";
978+
code += " using container_type = ";
979+
code += containerWithTypes;
980+
code += ";\n";
962981
}
963-
code += ">\n";
964-
code += "struct TypeHandler<Ctx, ";
965-
code += containerWithTypes;
966-
code += "> {\n";
982+
967983
code += " using DB = typename Ctx::DataBuffer;\n";
968984

969985
if (c.captureKeys) {
@@ -972,10 +988,6 @@ void genContainerTypeHandler(std::unordered_set<const ContainerInfo*>& used,
972988
code += " static constexpr bool captureKeys = false;\n";
973989
}
974990

975-
code += " using container_type = ";
976-
code += containerWithTypes;
977-
code += ";\n";
978-
979991
code += " using type = ";
980992
if (processors.empty()) {
981993
code += "types::st::Unit<DB>";
@@ -991,14 +1003,13 @@ void genContainerTypeHandler(std::unordered_set<const ContainerInfo*>& used,
9911003
}
9921004
code += ";\n";
9931005

1006+
code += c.codegen.scopedExtra;
1007+
9941008
code += " static types::st::Unit<DB> getSizeType(\n";
9951009
code += " Ctx& ctx,\n";
996-
code += " const ";
997-
code += containerWithTypes;
998-
code += "& container,\n";
999-
code += " typename TypeHandler<Ctx, ";
1000-
code += containerWithTypes;
1001-
code += ">::type returnArg) {\n";
1010+
code += " const container_type& container,\n";
1011+
code +=
1012+
" typename TypeHandler<Ctx, container_type>::type returnArg) {\n";
10021013
code += func; // has rubbish indentation
10031014
code += " }\n";
10041015

@@ -1029,7 +1040,6 @@ void genContainerTypeHandler(std::unordered_set<const ContainerInfo*>& used,
10291040
code += "},\n";
10301041
}
10311042
code += " };\n";
1032-
10331043
code += "};\n\n";
10341044
}
10351045

oi/ContainerInfo.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,10 @@ ContainerInfo::ContainerInfo(const fs::path& path) {
298298
codegenToml["extra"].value<std::string>()) {
299299
codegen.extra = std::move(*str);
300300
}
301+
if (std::optional<std::string> str =
302+
codegenToml["scoped_extra"].value<std::string>()) {
303+
codegen.scopedExtra = std::move(*str);
304+
}
301305

302306
if (toml::array* arr = codegenToml["processor"].as_array()) {
303307
codegen.processors.reserve(arr->size());

oi/ContainerInfo.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ struct ContainerInfo {
3838
std::string func;
3939
std::string traversalFunc = "";
4040
std::string extra = "";
41+
std::string scopedExtra = "";
4142
std::vector<Processor> processors{};
4243
};
4344

test/integration/std_variant.toml

Lines changed: 87 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ definitions = '''
99

1010
[cases]
1111
[cases.char_int64_1]
12-
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
1312
param_types = ["const std::variant<char, int64_t>&"]
1413
setup = "return 'a';"
1514
expect_json = '''[{
@@ -22,8 +21,17 @@ definitions = '''
2221
"members":[
2322
{"typeName":"int8_t", "staticSize":1, "exclusiveSize":1, "dynamicSize":0}
2423
]}]'''
24+
expect_json_v2 = '''[{
25+
"size":16,
26+
"staticSize":16,
27+
"exclusiveSize":15,
28+
"length":1,
29+
"capacity":1,
30+
"members":[
31+
{"typeNames":["int8_t"], "size":1, "staticSize":1, "exclusiveSize":1}
32+
]
33+
}]'''
2534
[cases.char_int64_2]
26-
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
2735
param_types = ["const std::variant<char, int64_t>&"]
2836
setup = "return 1234;"
2937
expect_json = '''[{
@@ -36,9 +44,18 @@ definitions = '''
3644
"members":[
3745
{"typeName":"int64_t", "staticSize":8, "exclusiveSize":8, "dynamicSize":0}
3846
]}]'''
47+
expect_json_v2 = '''[{
48+
"size":16,
49+
"staticSize":16,
50+
"exclusiveSize":8,
51+
"length":1,
52+
"capacity":1,
53+
"members":[
54+
{"typeNames":["int64_t"], "size":8, "staticSize":8, "exclusiveSize":8}
55+
]
56+
}]'''
3957

4058
[cases.vector_int_1]
41-
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
4259
param_types = ["const std::variant<std::vector<int>, int>&"]
4360
setup = "return std::vector<int>{1,2,3};"
4461
expect_json = '''[{
@@ -59,8 +76,21 @@ definitions = '''
5976
"elementStaticSize":4
6077
}
6178
]}]'''
79+
expect_json_v2 = '''[{
80+
"size":44,
81+
"staticSize":32,
82+
"exclusiveSize":8,
83+
"length":1,
84+
"capacity":1,
85+
"members":[
86+
{"typeNames":["std::vector<int32_t, std::allocator<int32_t>>"], "size":36, "staticSize":24, "exclusiveSize":24, "members":[
87+
{"typeNames":["int32_t"], "size":4, "staticSize":4, "exclusiveSize":4},
88+
{"typeNames":["int32_t"], "size":4, "staticSize":4, "exclusiveSize":4},
89+
{"typeNames":["int32_t"], "size":4, "staticSize":4, "exclusiveSize":4}
90+
]}
91+
]
92+
}]'''
6293
[cases.vector_int_2]
63-
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
6494
param_types = ["const std::variant<std::vector<int>, int>&"]
6595
setup = "return 123;"
6696
expect_json = '''[{
@@ -76,9 +106,18 @@ definitions = '''
76106
"dynamicSize":0
77107
}
78108
]}]'''
109+
expect_json_v2 = '''[{
110+
"size":32,
111+
"staticSize":32,
112+
"exclusiveSize":28,
113+
"length":1,
114+
"capacity":1,
115+
"members":[
116+
{"typeNames":["int32_t"], "size":4, "staticSize":4, "exclusiveSize":4}
117+
]
118+
}]'''
79119

80120
[cases.optional]
81-
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
82121
# This test case ensures that the alignment of std::variant is set
83122
# correctly, as otherwise the size of the std::optional would be wrong
84123
param_types = ["const std::optional<std::variant<char, int64_t>>&"]
@@ -103,9 +142,25 @@ definitions = '''
103142
]
104143
}
105144
]}]'''
145+
expect_json_v2 = '''[{
146+
"size":24,
147+
"staticSize":24,
148+
"exclusiveSize":8,
149+
"length":1,
150+
"capacity":1,
151+
"members":[{
152+
"size":16,
153+
"staticSize":16,
154+
"exclusiveSize":8,
155+
"length":1,
156+
"capacity":1,
157+
"members":[
158+
{"typeNames":["int64_t"], "size":8, "staticSize":8, "exclusiveSize":8}
159+
]
160+
}]
161+
}]'''
106162

107163
[cases.empty]
108-
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
109164
# https://en.cppreference.com/w/cpp/utility/variant/valueless_by_exception
110165
param_types = ["const std::variant<int, Thrower>&"]
111166
setup = '''
@@ -127,13 +182,20 @@ definitions = '''
127182
"elementStaticSize":4,
128183
"NOT":"members"
129184
}]'''
185+
expect_json_v2 = '''[{
186+
"size":8,
187+
"staticSize":8,
188+
"exclusiveSize":8,
189+
"length":0,
190+
"capacity":1,
191+
"members":[]
192+
}]'''
130193

131194
# With less than 256 options, parameter indexes are stored in a uint8_t and
132195
# the invalid index value is 0xff. These tests check that we regonise that
133196
# 0xff can be a valid index if there are at least 256 parameters, and that
134197
# the invalid index value is raised to 0xffff.
135198
[cases.256_params_256]
136-
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
137199
param_types = ["const std::variant<Thrower,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,char>&"]
138200
setup = "return 'a';"
139201
expect_json = '''[{
@@ -146,8 +208,17 @@ definitions = '''
146208
"members":[
147209
{"typeName":"int8_t", "staticSize":1, "exclusiveSize":1, "dynamicSize":0}
148210
]}]'''
211+
expect_json_v2 = '''[{
212+
"size":8,
213+
"staticSize":8,
214+
"exclusiveSize":7,
215+
"length":1,
216+
"capacity":1,
217+
"members":[
218+
{"typeNames":["int8_t"], "size":1, "staticSize":1, "exclusiveSize":1}
219+
]
220+
}]'''
149221
[cases.256_params_empty]
150-
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
151222
param_types = ["const std::variant<Thrower,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,char>&"]
152223
setup = '''
153224
std::variant<Thrower,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,char> var{'a'};
@@ -167,3 +238,11 @@ definitions = '''
167238
"elementStaticSize":4,
168239
"NOT":"members"
169240
}]'''
241+
expect_json_v2 = '''[{
242+
"size":8,
243+
"staticSize":8,
244+
"exclusiveSize":8,
245+
"length":0,
246+
"capacity":1,
247+
"members":[]
248+
}]'''

types/README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ This document describes the format of the container definition files contained i
5555
`static types::st::Unit<DB> getSizeType(const T& container, ST returnArg)`
5656
where `ST` defines the combined static type of each supplied processor.
5757

58+
- `scoped_extra`
59+
60+
Extra C++ code to be included in the generated `TypeHandler` for this
61+
container.
62+
5863
- `extra`
5964

6065
Any extra C++ code to be included directly. This code is not automatically

types/std_variant.toml

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,54 @@ void getSizeType(const %1%<Types...> &container, size_t& returnArg)
3636
}
3737
"""
3838

39-
# TODO: Add tbv2 definitions. The removed intermediate handler is a good
40-
# template for this, find it in the git logs.
39+
scoped_extra = """
40+
template <size_t I = 0>
41+
static types::st::Unit<DB>
42+
getSizeTypeRecursive(
43+
Ctx& ctx,
44+
const container_type& container,
45+
typename TypeHandler<Ctx, container_type>::type returnArg
46+
) {
47+
if constexpr (I < sizeof...(Types)) {
48+
if (I == container.index()) {
49+
return returnArg.template delegate<I>([&ctx, &container](auto ret) {
50+
return OIInternal::getSizeType<Ctx>(ctx, std::get<I>(container), ret);
51+
});
52+
} else {
53+
return getSizeTypeRecursive<I+1>(ctx, container, returnArg);
54+
}
55+
} else {
56+
return returnArg.template delegate<sizeof...(Types)>(std::identity());
57+
}
58+
}
59+
"""
60+
61+
handler_header = """
62+
template <typename Ctx, typename... Types>
63+
struct TypeHandler<Ctx, std::variant<Types...>>
64+
"""
65+
66+
traversal_func = """
67+
return getSizeTypeRecursive(ctx, container, returnArg);
68+
"""
69+
70+
[[codegen.processor]]
71+
type = "types::st::Sum<DB, typename TypeHandler<Ctx, Types>::type..., types::st::Unit<DB>>"
72+
func = """
73+
static constexpr std::array<inst::Field, sizeof...(Types)> children{
74+
make_field<Ctx, Types>("*")...,
75+
};
76+
77+
auto sum = std::get<ParsedData::Sum>(d.val);
78+
79+
el.container_stats = result::Element::ContainerStats {
80+
.capacity = 1,
81+
.length = sum.index == sizeof...(Types) ? 0u : 1u,
82+
};
83+
84+
if (el.container_stats->length == 0)
85+
return;
86+
87+
el.exclusive_size -= children[sum.index].static_size;
88+
stack_ins(children[sum.index]);
89+
"""

0 commit comments

Comments
 (0)