Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add #[packed] attribute to create packed structs #5758

Closed
wants to merge 4 commits into from

Conversation

huonw
Copy link
Member

@huonw huonw commented Apr 6, 2013

This adds a #[packed] attribute for structs, like GCC's __attribute__((packed)), e.g.

#[packed]
struct Size5 {
   a: u8,
   b: u32
}

It works on normal and tuple structs, but is (silently) ignored on enums.

Notes:

  • I couldn't work out a better way to store the packed-ness of a struct than in ty::sty.
  • I have no idea if this works on any architecture other than x86-64 and i686 (although there's no particular reason why it wouldn't, it's just using the packed attribute in LLVM)
  • I feel like there could be more tests, but I'm not really sure what/how to test.

Closes #1704.

(I'm not sure about the XXX in cabi_mips.rs, so merging this should probably wait.) Resolved!

@@ -174,7 +185,8 @@ fn struct_ty(ty: TypeRef,
fields.push(ty);
}

return T_struct(fields);
// XXX: Is this meant to be always non-packed?
return T_struct(fields, false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crabtw, git blame seems to say you wrote much of the mips backend.

I don't understand the purpose of the struct being created by struct_ty (something about passing arguments to a function since it is called from classify_arg_ty?). Is this struct always meant to be non-packed, even if the argument being classified is a packed struct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbaupp struct_ty is used to ensure argument is placed in correct offset. The returned struct is always non-packed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

A struct (inc. tuple struct) can be annotated with #[packed], so that there
is no padding between its elements, like GCC's `__attribute__((packed))`.

Closes rust-lang#1704.
@nikomatsakis
Copy link
Contributor

This is good work but I think the approach isn't quite congruent with how we normally do things. Whether or not a struct is packed is really a detail of its representation, and is not a concern of the type system, so I think we should not modify ty_struct.

There are two common patterns to this approach.

One is to add a side table in the type context (indexed by the struct's def-id) to indicate whether or not a struct is packed. Typically, for things local to the crate, these side tables are populated during the "collect" phase of type check (i.e., typeck/collect.rs). For structs found in external crates, a bit of data is stuffed into the metadata and a new routine is added to csearch. An example of this pattern is the cx.supertraits table, which is queried by trait_supertraits.

However, a cache might be overkill, since the "packed" information is only relevant to type_of (I think?) and there is already a cache in type_of to prevent us from processing the same struct over and over, so perhaps a better pattern to follow would be provided_trait_methods(), which avoids building up any data structure. Instead, if the struct is defined within the current crate, the accessor could use the ast map and check the attributes. If the struct is defined within another crate, the accessor would use csearch to decode the information out of the crate metadata.

@huonw
Copy link
Member Author

huonw commented Apr 10, 2013

Hm, thanks for the explanation. I'll try to fix that some time in the next week or so.

@huonw huonw closed this Apr 10, 2013
bors added a commit that referenced this pull request Apr 10, 2013
#5758 take 2.

This adds a `#[packed]` attribute for structs, like GCC's `__attribute__((packed))`, e.g.

```rust
#[packed]
struct Size5 {
   a: u8,
   b: u32
}
```

It works on normal and tuple structs, but is (silently) ignored on enums.

Closes #1704.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 14, 2020
Require `or_patterns` to suggest nesting them

changelog: Require `#![feature(or_patterns)]` to trigger [`unnested_or_patterns`]

Fixes rust-lang#5704
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants