CodeGen: Don't measure the sizes of union members

In general, we can't tell which member is active in a union so it is not
safe to try and measure any of them.

Explicitly set the alignment of unions (and structs/classes) in CodeGen
if it is available, as the C++ compiler can no longer infer it from the
members.
This commit is contained in:
Alastair Robertson 2023-07-19 06:45:28 -07:00 committed by Alastair Robertson
parent 2d1cc92bb4
commit 884b9a6e95
6 changed files with 35 additions and 8 deletions

View File

@ -20,21 +20,21 @@ workflows:
- build-gcc - build-gcc
oid_test_args: "-ftype-graph" oid_test_args: "-ftype-graph"
tests_regex: "OidIntegration\\..*" tests_regex: "OidIntegration\\..*"
exclude_regex: ".*inheritance_polymorphic.*|.*pointers_incomplete_containing_struct|.*arrays_member_int0|OidIntegration.unions.*|.*thrift_unions_dynamic_.*" exclude_regex: ".*inheritance_polymorphic.*|.*pointers_incomplete_containing_struct|.*arrays_member_int0"
- test: - test:
name: test-typed-data-segment-gcc name: test-typed-data-segment-gcc
requires: requires:
- build-gcc - build-gcc
oid_test_args: "-ftyped-data-segment" oid_test_args: "-ftyped-data-segment"
tests_regex: "OidIntegration\\..*" tests_regex: "OidIntegration\\..*"
exclude_regex: ".*inheritance_polymorphic.*|.*pointers.*|.*arrays_member_int0|.*cycles_.*|OidIntegration.unions.*|.*thrift_unions_dynamic_.*" exclude_regex: ".*inheritance_polymorphic.*|.*pointers.*|.*arrays_member_int0|.*cycles_.*"
- test: - test:
name: test-tree-builder-type-checking-gcc name: test-tree-builder-type-checking-gcc
requires: requires:
- build-gcc - build-gcc
oid_test_args: "-ftree-builder-type-checking" oid_test_args: "-ftree-builder-type-checking"
tests_regex: "OidIntegration\\..*" tests_regex: "OidIntegration\\..*"
exclude_regex: ".*inheritance_polymorphic.*|.*pointers.*|.*arrays_member_int0|.*cycles_.*|OidIntegration.unions.*|.*thrift_unions_dynamic_.*" exclude_regex: ".*inheritance_polymorphic.*|.*pointers.*|.*arrays_member_int0|.*cycles_.*"
- coverage: - coverage:
name: coverage name: coverage
requires: requires:
@ -68,7 +68,7 @@ workflows:
oid_test_args: "-ftype-graph" oid_test_args: "-ftype-graph"
tests_regex: "OidIntegration\\..*" tests_regex: "OidIntegration\\..*"
# Tests disabled due to bad DWARF generated by the old clang compiler in CI # Tests disabled due to bad DWARF generated by the old clang compiler in CI
exclude_regex: ".*inheritance_polymorphic.*|.*pointers_incomplete_containing_struct|.*arrays_member_int0|.*fbstring.*|.*std_string_*|.*multi_arg_tb_.*|.*ignored_a|OidIntegration.unions.*|.*thrift_unions_dynamic_.*" exclude_regex: ".*inheritance_polymorphic.*|.*pointers_incomplete_containing_struct|.*arrays_member_int0|.*fbstring.*|.*std_string_*|.*multi_arg_tb_.*|.*ignored_a"
executors: executors:
ubuntu-docker: ubuntu-docker:

View File

@ -251,6 +251,13 @@ void genDefsClass(const Class& c, std::string& code) {
code += "__attribute__((__packed__)) "; code += "__attribute__((__packed__)) ";
} }
if (c.kind() == Class::Kind::Union) {
// Need to specify alignment manually for unions as their members have been
// removed. It would be nice to do this for all types, but our alignment
// information is not complete, so it would result in some errors.
code += "alignas(" + std::to_string(c.align()) + ") ";
}
code += c.name() + " {\n"; code += c.name() + " {\n";
for (const auto& mem : c.members) { for (const auto& mem : c.members) {
code += " " + mem.type().name() + " " + mem.name; code += " " + mem.type().name() + " " + mem.name;

View File

@ -41,6 +41,12 @@ void RemoveMembers::accept(Type& type) {
} }
void RemoveMembers::visit(Class& c) { void RemoveMembers::visit(Class& c) {
if (c.kind() == Class::Kind::Union) {
// In general, we can't tell which member is active in a union, so it is not
// safe to try and measure any of them.
c.members.clear();
}
for (const auto& param : c.templateParams) { for (const auto& param : c.templateParams) {
accept(param.type()); accept(param.type());
} }

View File

@ -28,6 +28,8 @@ namespace type_graph {
* *
* Removes members as requested by the [[codegen.ignore]] section in the OI * Removes members as requested by the [[codegen.ignore]] section in the OI
* config. * config.
*
* Removes members from unions as we have no way of telling which one is active.
*/ */
class RemoveMembers : public RecursiveVisitor { class RemoveMembers : public RecursiveVisitor {
public: public:

View File

@ -201,9 +201,8 @@ TEST(CodeGenTest, UnionMembersAlignment) {
)", )",
R"( R"(
[0] Union: MyUnion_0 (size: 8, align: 8) [0] Union: MyUnion_0 (size: 8, align: 8)
Member: a_0 (offset: 0, align: 1) Member: __oi_padding_0 (offset: 0)
Primitive: int8_t [1] Array: (length: 8)
Member: b_1 (offset: 8, align: 8) Primitive: int8_t
Primitive: int64_t
)"); )");
} }

View File

@ -185,3 +185,16 @@ TEST(RemoveMembersTest, RecurseClassChild) {
Primitive: int32_t Primitive: int32_t
)"); )");
} }
TEST(RemoveMembersTest, Union) {
test(RemoveMembers::createPass({}), R"(
[0] Union: MyUnion (size: 4)
Member: a (offset: 0)
Primitive: int32_t
Member: b (offset: 0)
Primitive: int32_t
)",
R"(
[0] Union: MyUnion (size: 4)
)");
}