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
This commit is contained in:
Jake Hillion 2024-02-23 14:49:06 +00:00
parent 7103680894
commit 9289fbe8d8
6 changed files with 98 additions and 36 deletions

View File

@ -941,29 +941,45 @@ void genContainerTypeHandler(std::unordered_set<const ContainerInfo*>& used,
containerWithTypes = "OICaptureKeys<" + containerWithTypes + ">";
}
code += "template <typename Ctx";
types = 0, values = 0;
for (const auto& p : templateParams) {
if (p.value) {
code += ", ";
// TODO: This is tech debt. This should be moved to a field in the container
// spec/`.toml` called something like `codegen.handler_header` or have an
// explicit option for variable template parameters. However I'm landing it
// anyway to demonstrate how to handle tagged unions in TreeBuilder-v2.
if (c.typeName == "std::variant") {
code += R"(
template <typename Ctx, typename... Types>
struct TypeHandler<Ctx, std::variant<Types...>> {
using container_type = std::variant<Types...>;
)";
} else {
code += "template <typename Ctx";
types = 0, values = 0;
for (const auto& p : templateParams) {
if (p.value) {
code += ", ";
// HACK: forward all enums directly. this might turn out to be a problem
// if there are enums we should be regenerating/use in the body.
if (const auto* e = dynamic_cast<const Enum*>(&p.type())) {
code += e->inputName();
// HACK: forward all enums directly. this might turn out to be a problem
// if there are enums we should be regenerating/use in the body.
if (const auto* e = dynamic_cast<const Enum*>(&p.type())) {
code += e->inputName();
} else {
code += p.type().name();
}
code += " N" + std::to_string(values++);
} else {
code += p.type().name();
code += ", typename T" + std::to_string(types++);
}
code += " N" + std::to_string(values++);
} else {
code += ", typename T" + std::to_string(types++);
}
code += ">\n";
code += "struct TypeHandler<Ctx, ";
code += containerWithTypes;
code += "> {\n";
code += " using container_type = ";
code += containerWithTypes;
code += ";\n";
}
code += ">\n";
code += "struct TypeHandler<Ctx, ";
code += containerWithTypes;
code += "> {\n";
code += " using DB = typename Ctx::DataBuffer;\n";
if (c.captureKeys) {
@ -972,10 +988,6 @@ void genContainerTypeHandler(std::unordered_set<const ContainerInfo*>& used,
code += " static constexpr bool captureKeys = false;\n";
}
code += " using container_type = ";
code += containerWithTypes;
code += ";\n";
code += " using type = ";
if (processors.empty()) {
code += "types::st::Unit<DB>";
@ -991,11 +1003,11 @@ void genContainerTypeHandler(std::unordered_set<const ContainerInfo*>& used,
}
code += ";\n";
code += c.codegen.scopedExtra;
code += " static types::st::Unit<DB> getSizeType(\n";
code += " Ctx& ctx,\n";
code += " const ";
code += containerWithTypes;
code += "& container,\n";
code += " const container_type& container,\n";
code += " typename TypeHandler<Ctx, ";
code += containerWithTypes;
code += ">::type returnArg) {\n";
@ -1029,7 +1041,6 @@ void genContainerTypeHandler(std::unordered_set<const ContainerInfo*>& used,
code += "},\n";
}
code += " };\n";
code += "};\n\n";
}

View File

@ -298,6 +298,10 @@ ContainerInfo::ContainerInfo(const fs::path& path) {
codegenToml["extra"].value<std::string>()) {
codegen.extra = std::move(*str);
}
if (std::optional<std::string> str =
codegenToml["scoped_extra"].value<std::string>()) {
codegen.scopedExtra = std::move(*str);
}
if (toml::array* arr = codegenToml["processor"].as_array()) {
codegen.processors.reserve(arr->size());

View File

@ -38,6 +38,7 @@ struct ContainerInfo {
std::string func;
std::string traversalFunc = "";
std::string extra = "";
std::string scopedExtra = "";
std::vector<Processor> processors{};
};

View File

@ -9,7 +9,6 @@ definitions = '''
[cases]
[cases.char_int64_1]
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
param_types = ["const std::variant<char, int64_t>&"]
setup = "return 'a';"
expect_json = '''[{
@ -23,7 +22,6 @@ definitions = '''
{"typeName":"int8_t", "staticSize":1, "exclusiveSize":1, "dynamicSize":0}
]}]'''
[cases.char_int64_2]
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
param_types = ["const std::variant<char, int64_t>&"]
setup = "return 1234;"
expect_json = '''[{
@ -38,7 +36,6 @@ definitions = '''
]}]'''
[cases.vector_int_1]
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
param_types = ["const std::variant<std::vector<int>, int>&"]
setup = "return std::vector<int>{1,2,3};"
expect_json = '''[{
@ -60,7 +57,6 @@ definitions = '''
}
]}]'''
[cases.vector_int_2]
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
param_types = ["const std::variant<std::vector<int>, int>&"]
setup = "return 123;"
expect_json = '''[{
@ -78,7 +74,6 @@ definitions = '''
]}]'''
[cases.optional]
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
# This test case ensures that the alignment of std::variant is set
# correctly, as otherwise the size of the std::optional would be wrong
param_types = ["const std::optional<std::variant<char, int64_t>>&"]
@ -105,7 +100,6 @@ definitions = '''
]}]'''
[cases.empty]
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
# https://en.cppreference.com/w/cpp/utility/variant/valueless_by_exception
param_types = ["const std::variant<int, Thrower>&"]
setup = '''
@ -133,7 +127,6 @@ definitions = '''
# 0xff can be a valid index if there are at least 256 parameters, and that
# the invalid index value is raised to 0xffff.
[cases.256_params_256]
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
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>&"]
setup = "return 'a';"
expect_json = '''[{
@ -147,7 +140,6 @@ definitions = '''
{"typeName":"int8_t", "staticSize":1, "exclusiveSize":1, "dynamicSize":0}
]}]'''
[cases.256_params_empty]
oil_skip = "std::variant is not implemented for treebuilder v2" # https://github.com/facebookexperimental/object-introspection/issues/298
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>&"]
setup = '''
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'};

View File

@ -55,6 +55,11 @@ This document describes the format of the container definition files contained i
`static types::st::Unit<DB> getSizeType(const T& container, ST returnArg)`
where `ST` defines the combined static type of each supplied processor.
- `scoped_extra`
Extra C++ code to be included in the generated `TypeHandler` for this
container.
- `extra`
Any extra C++ code to be included directly. This code is not automatically

View File

@ -36,5 +36,54 @@ void getSizeType(const %1%<Types...> &container, size_t& returnArg)
}
"""
# TODO: Add tbv2 definitions. The removed intermediate handler is a good
# template for this, find it in the git logs.
scoped_extra = """
template <size_t I = 0>
static types::st::Unit<DB>
getSizeTypeRecursive(
Ctx& ctx,
const typename container_type& container,
typename TypeHandler<Ctx, typename container_type>::type returnArg
) {
if constexpr (I < sizeof...(Types)) {
if (I == container.index()) {
return returnArg.template delegate<I>([&ctx, &container](auto ret) {
return OIInternal::getSizeType<Ctx>(ctx, std::get<I>(container), ret);
});
} else {
return getSizeTypeRecursive<I+1>(ctx, container, returnArg);
}
} else {
return returnArg.template delegate<sizeof...(Types)>(std::identity());
}
}
"""
handler_header = """
template <typename Ctx, typename... Types>
struct TypeHandler<Ctx, std::variant<Types...>>
"""
traversal_func = """
return getSizeTypeRecursive(ctx, container, returnArg);
"""
[[codegen.processor]]
type = "types::st::Sum<Ctx, typename TypeHandler<DB, Types>::type..., types::st::Unit<DB>>"
func = """
static constexpr std::array<inst::Field, sizeof...(Types)> children{
make_field<Ctx, Types>("*")...,
};
auto sum = std::get<ParsedData::Sum>(d.val);
el.container_stats = result::Element::ContainerStats {
.capacity = 1,
.length = sum.index == sizeof...(Types) ? 0 : 1,
};
if (el.container_stats->length == 0)
return;
el.exclusive_size -= children[sum.index].static_size;
stack_ins(children[sum.index]);
"""