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

[beta] trans: pad const structs to aligned size #37344

Merged
merged 1 commit into from
Oct 22, 2016

Conversation

TimNN
Copy link
Contributor

@TimNN TimNN commented Oct 22, 2016

This fixes #37222 on beta.

As pointed out by @eddyb in #37281 (comment), the nightly fix cannot be backported as-is due to other changes.

An alternative to this adapted fix would be to backport #36904 as well, in addition to #37281.

Also cc @rust-lang/compiler, since this is not a straight backport.

@rust-highfive
Copy link
Collaborator

r? @Aatch

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

warning Warning warning

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@TimNN
Copy link
Contributor Author

TimNN commented Oct 22, 2016

Since there's no CI here I'll note that this passed bootstrap.py --step check --stage 2 on x86_64-unknown-linux-gnu.

@sfackler sfackler added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 22, 2016
@eddyb
Copy link
Member

eddyb commented Oct 22, 2016

Why not add an alignment argument to build_const_struct?

@TimNN
Copy link
Contributor Author

TimNN commented Oct 22, 2016

I saw how the padding was handled for layout::General { discr: d, ref variants, .. } => and just applied the same logic to the other two call sites of build_const_struct. I suppose just passing the alignment to build_const_struct is the cleaner solution. I'll see about fixing that.

@TimNN TimNN force-pushed the beta-pad-align branch 3 times, most recently from 1a7f6c0 to 6343055 Compare October 22, 2016 16:32
@eddyb
Copy link
Member

eddyb commented Oct 22, 2016

@nikomatsakis @brson r=me

@alexcrichton
Copy link
Member

Merging as #37281 is beta-accepted as well. Thanks so much for being on top of this @TimNN!

@alexcrichton alexcrichton merged commit f1e6e7e into rust-lang:beta Oct 22, 2016
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 22, 2016
@TimNN TimNN deleted the beta-pad-align branch October 22, 2016 23:24
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.

6 participants