Skip to content

Commit

Permalink
Merge #1188
Browse files Browse the repository at this point in the history
1188: Support align and packed repr layout on structs r=dafaust a=dafaust

This is a start at handling the various layout options supported by Rust, beginning with `#[repr(align(N))]` and `#[repr(packed(N))]`, on structs and tuple structs.

There are several other layout options which remain to be supported such as `#[repr(C)]`, `#[repr(transparent)]`, combinations e.g. `#[repr(C, packed(2))]`, as well as layouts on union and enum types.

Fixes: #915 

Co-authored-by: David Faust <david.faust@oracle.com>
  • Loading branch information
bors[bot] and dafaust authored Apr 29, 2022
2 parents f8a137b + fd51331 commit f38bf60
Show file tree
Hide file tree
Showing 12 changed files with 223 additions and 5 deletions.
20 changes: 20 additions & 0 deletions gcc/rust/backend/rust-compile-type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,26 @@ TyTyResolveCompile::visit (const TyTy::ADTType &type)
type_record = ctx->get_backend ()->union_type (enum_fields);
}

// Handle repr options
// TODO: "packed" should only narrow type alignment and "align" should only
// widen it. Do we need to check and enforce this here, or is it taken care of
// later on in the gcc middle-end?
TyTy::ADTType::ReprOptions repr = type.get_repr_options ();
if (repr.pack)
{
TYPE_PACKED (type_record);
if (repr.pack > 1)
{
SET_TYPE_ALIGN (type_record, repr.pack * 8);
TYPE_USER_ALIGN (type_record) = 1;
}
}
else if (repr.align)
{
SET_TYPE_ALIGN (type_record, repr.align * 8);
TYPE_USER_ALIGN (type_record) = 1;
}

std::string named_struct_str
= type.get_ident ().path.get () + type.subst_as_string ();
tree named_struct
Expand Down
67 changes: 67 additions & 0 deletions gcc/rust/typecheck/rust-hir-type-check-base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -260,5 +260,72 @@ TypeCheckBase::resolve_literal (const Analysis::NodeMapping &expr_mappings,
return infered;
}

TyTy::ADTType::ReprOptions
TypeCheckBase::parse_repr_options (const AST::AttrVec &attrs, Location locus)
{
TyTy::ADTType::ReprOptions repr;
repr.pack = 0;
repr.align = 0;

for (const auto &attr : attrs)
{
bool is_repr = attr.get_path ().as_string ().compare ("repr") == 0;
if (is_repr)
{
const AST::AttrInput &input = attr.get_attr_input ();
bool is_token_tree = input.get_attr_input_type ()
== AST::AttrInput::AttrInputType::TOKEN_TREE;
rust_assert (is_token_tree);
const auto &option = static_cast<const AST::DelimTokenTree &> (input);
AST::AttrInputMetaItemContainer *meta_items
= option.parse_to_meta_item ();

const std::string inline_option
= meta_items->get_items ().at (0)->as_string ();

// TODO: it would probably be better to make the MetaItems more aware
// of constructs with nesting like #[repr(packed(2))] rather than
// manually parsing the string "packed(2)" here.

size_t oparen = inline_option.find ('(', 0);
bool is_pack = false, is_align = false;
unsigned char value = 1;

if (oparen == std::string::npos)
{
is_pack = inline_option.compare ("packed") == 0;
is_align = inline_option.compare ("align") == 0;
}

else
{
std::string rep = inline_option.substr (0, oparen);
is_pack = rep.compare ("packed") == 0;
is_align = rep.compare ("align") == 0;

size_t cparen = inline_option.find (')', oparen);
if (cparen == std::string::npos)
{
rust_error_at (locus, "malformed attribute");
}

std::string value_str = inline_option.substr (oparen, cparen);
value = strtoul (value_str.c_str () + 1, NULL, 10);
}

if (is_pack)
repr.pack = value;
else if (is_align)
repr.align = value;

// Multiple repr options must be specified with e.g. #[repr(C,
// packed(2))].
break;
}
}

return repr;
}

} // namespace Resolver
} // namespace Rust
3 changes: 3 additions & 0 deletions gcc/rust/typecheck/rust-hir-type-check-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ class TypeCheckBase : public HIR::HIRFullVisitorBase
TyTy::BaseType *resolve_literal (const Analysis::NodeMapping &mappings,
HIR::Literal &literal, Location locus);

TyTy::ADTType::ReprOptions parse_repr_options (const AST::AttrVec &attrs,
Location locus);

Analysis::Mappings *mappings;
Resolver *resolver;
TypeCheckContext *context;
Expand Down
16 changes: 14 additions & 2 deletions gcc/rust/typecheck/rust-hir-type-check-stmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,18 @@ class TypeCheckStmt : public TypeCheckBase
TyTy::VariantDef::VariantType::TUPLE, nullptr,
std::move (fields)));

// Process #[repr(...)] attribute, if any
const AST::AttrVec &attrs = struct_decl.get_outer_attrs ();
TyTy::ADTType::ReprOptions repr
= parse_repr_options (attrs, struct_decl.get_locus ());

TyTy::BaseType *type
= new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
mappings->get_next_hir_id (),
struct_decl.get_identifier (), ident,
TyTy::ADTType::ADTKind::TUPLE_STRUCT,
std::move (variants), std::move (substitutions));
std::move (variants), std::move (substitutions),
repr);

context->insert_type (struct_decl.get_mappings (), type);
infered = type;
Expand Down Expand Up @@ -311,12 +317,18 @@ class TypeCheckStmt : public TypeCheckBase
TyTy::VariantDef::VariantType::STRUCT, nullptr,
std::move (fields)));

// Process #[repr(...)] attribute, if any
const AST::AttrVec &attrs = struct_decl.get_outer_attrs ();
TyTy::ADTType::ReprOptions repr
= parse_repr_options (attrs, struct_decl.get_locus ());

TyTy::BaseType *type
= new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
mappings->get_next_hir_id (),
struct_decl.get_identifier (), ident,
TyTy::ADTType::ADTKind::STRUCT_STRUCT,
std::move (variants), std::move (substitutions));
std::move (variants), std::move (substitutions),
repr);

context->insert_type (struct_decl.get_mappings (), type);
infered = type;
Expand Down
16 changes: 14 additions & 2 deletions gcc/rust/typecheck/rust-hir-type-check-toplevel.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,18 @@ class TypeCheckTopLevel : public TypeCheckBase
TyTy::VariantDef::VariantType::TUPLE, nullptr,
std::move (fields)));

// Process #[repr(X)] attribute, if any
const AST::AttrVec &attrs = struct_decl.get_outer_attrs ();
TyTy::ADTType::ReprOptions repr
= parse_repr_options (attrs, struct_decl.get_locus ());

TyTy::BaseType *type
= new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
mappings->get_next_hir_id (),
struct_decl.get_identifier (), ident,
TyTy::ADTType::ADTKind::TUPLE_STRUCT,
std::move (variants), std::move (substitutions));
std::move (variants), std::move (substitutions),
repr);

context->insert_type (struct_decl.get_mappings (), type);
}
Expand Down Expand Up @@ -196,12 +202,18 @@ class TypeCheckTopLevel : public TypeCheckBase
TyTy::VariantDef::VariantType::STRUCT, nullptr,
std::move (fields)));

// Process #[repr(X)] attribute, if any
const AST::AttrVec &attrs = struct_decl.get_outer_attrs ();
TyTy::ADTType::ReprOptions repr
= parse_repr_options (attrs, struct_decl.get_locus ());

TyTy::BaseType *type
= new TyTy::ADTType (struct_decl.get_mappings ().get_hirid (),
mappings->get_next_hir_id (),
struct_decl.get_identifier (), ident,
TyTy::ADTType::ADTKind::STRUCT_STRUCT,
std::move (variants), std::move (substitutions));
std::move (variants), std::move (substitutions),
repr);

context->insert_type (struct_decl.get_mappings (), type);
}
Expand Down
3 changes: 2 additions & 1 deletion gcc/rust/typecheck/rust-tyty.cc
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,8 @@ ADTType::clone () const

return new ADTType (get_ref (), get_ty_ref (), identifier, ident,
get_adt_kind (), cloned_variants, clone_substs (),
used_arguments, get_combined_refs ());
get_repr_options (), used_arguments,
get_combined_refs ());
}

static bool
Expand Down
28 changes: 28 additions & 0 deletions gcc/rust/typecheck/rust-tyty.h
Original file line number Diff line number Diff line change
Expand Up @@ -1254,6 +1254,20 @@ class ADTType : public BaseType, public SubstitutionRef
ENUM
};

// Representation options, specified via attributes e.g. #[repr(packed)]
struct ReprOptions
{
// bool is_c;
// bool is_transparent;
//...

// For align and pack: 0 = unspecified. Nonzero = byte alignment.
// It is an error for both to be nonzero, this should be caught when
// parsing the #[repr] attribute.
unsigned char align = 0;
unsigned char pack = 0;
};

ADTType (HirId ref, std::string identifier, RustIdent ident, ADTKind adt_kind,
std::vector<VariantDef *> variants,
std::vector<SubstitutionParamMapping> subst_refs,
Expand All @@ -1276,7 +1290,20 @@ class ADTType : public BaseType, public SubstitutionRef
identifier (identifier), variants (variants), adt_kind (adt_kind)
{}

ADTType (HirId ref, HirId ty_ref, std::string identifier, RustIdent ident,
ADTKind adt_kind, std::vector<VariantDef *> variants,
std::vector<SubstitutionParamMapping> subst_refs, ReprOptions repr,
SubstitutionArgumentMappings generic_arguments
= SubstitutionArgumentMappings::error (),
std::set<HirId> refs = std::set<HirId> ())
: BaseType (ref, ty_ref, TypeKind::ADT, ident, refs),
SubstitutionRef (std::move (subst_refs), std::move (generic_arguments)),
identifier (identifier), variants (variants), adt_kind (adt_kind),
repr (repr)
{}

ADTKind get_adt_kind () const { return adt_kind; }
ReprOptions get_repr_options () const { return repr; }

bool is_struct_struct () const { return adt_kind == STRUCT_STRUCT; }
bool is_tuple_struct () const { return adt_kind == TUPLE_STRUCT; }
Expand Down Expand Up @@ -1385,6 +1412,7 @@ class ADTType : public BaseType, public SubstitutionRef
std::string identifier;
std::vector<VariantDef *> variants;
ADTType::ADTKind adt_kind;
ReprOptions repr;
};

class FnType : public BaseType, public SubstitutionRef
Expand Down
1 change: 1 addition & 0 deletions gcc/rust/util/rust-attributes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ static const BuiltinAttrDefinition __definitions[] = {
{"lang", HIR_LOWERING},
{"link_section", CODE_GENERATION},
{"no_mangle", CODE_GENERATION},
{"repr", CODE_GENERATION},
};

BuiltinAttributeMappings *
Expand Down
19 changes: 19 additions & 0 deletions gcc/testsuite/rust/compile/struct_align1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#[repr(align(8))]
struct Foo {
x: i16,
// { dg-warning "field is never read" "" { target *-*-* } .-1 }
y: i8,
// { dg-warning "field is never read" "" { target *-*-* } .-1 }
z: i32,
// { dg-warning "field is never read" "" { target *-*-* } .-1 }
}

#[repr(align(8))]
struct Bar(i8, i32);

fn main () {
let f = Foo { x: 5, y: 2, z: 13 };
// { dg-warning "unused name" "" { target *-*-* } .-1 }
let b = Bar (7, 262);
// { dg-warning "unused name" "" { target *-*-* } .-1 }
}
18 changes: 18 additions & 0 deletions gcc/testsuite/rust/compile/struct_align2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@

fn main () {

#[repr(align(8))]
struct Baz {
x: u16,
y: u32,
};

#[repr(align(4))]
struct Qux (u8, i16);

let b = Baz { x: 5, y: 1984 };
// { dg-warning "unused name" "" { target *-*-* } .-1 }

let c = Qux (1, 2);
// { dg-warning "unused name" "" { target *-*-* } .-1 }
}
19 changes: 19 additions & 0 deletions gcc/testsuite/rust/compile/struct_pack1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#[repr(packed(2))]
struct Foo {
x: i16,
// { dg-warning "field is never read" "" { target *-*-* } .-1 }
y: i8,
// { dg-warning "field is never read" "" { target *-*-* } .-1 }
z: i32,
// { dg-warning "field is never read" "" { target *-*-* } .-1 }
}

#[repr(packed)]
struct Bar(i8, i32);

fn main () {
let f = Foo { x: 5, y: 2, z: 13 };
// { dg-warning "unused name" "" { target *-*-* } .-1 }
let b = Bar (7, 262);
// { dg-warning "unused name" "" { target *-*-* } .-1 }
}
18 changes: 18 additions & 0 deletions gcc/testsuite/rust/compile/struct_pack2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@

fn main () {

#[repr(packed(2))]
struct Baz {
x: u16,
y: u32,
};

#[repr(packed)]
struct Qux (u8, i16);

let b = Baz { x: 5, y: 1984 };
// { dg-warning "unused name" "" { target *-*-* } .-1 }

let c = Qux (1, 2);
// { dg-warning "unused name" "" { target *-*-* } .-1 }
}

0 comments on commit f38bf60

Please sign in to comment.