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

rustc: ensure optimized enums have a properly aligned size. #46808

Merged
merged 1 commit into from Dec 18, 2017

Conversation

Projects
None yet
7 participants
@eddyb
Member

eddyb commented Dec 18, 2017

Fixes #46769 by padding the optimized enums wrapping packed data as necessary.

Note that this is not the only way to solve this - on nightly, #46436 makes it easier to fix without adding new padding because of the replacement of packed flags with a non-redundant scheme.
But because it can't be backported, the optimal fix will be in a separate nightly-only PR (#46809).

@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Dec 18, 2017

Collaborator

r? @estebank

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

Collaborator

rust-highfive commented Dec 18, 2017

r? @estebank

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

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb
Member

eddyb commented Dec 18, 2017

@arielb1

This comment has been minimized.

Show comment
Hide comment
@arielb1

arielb1 Dec 18, 2017

Contributor

@bors r+

Contributor

arielb1 commented Dec 18, 2017

@bors r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Dec 18, 2017

Contributor

📌 Commit b67bf2e has been approved by arielb1

Contributor

bors commented Dec 18, 2017

📌 Commit b67bf2e has been approved by arielb1

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Dec 18, 2017

Member

I've redone the test such that it fails (on beta & nightly) even without the ICE from #46623.

Member

eddyb commented Dec 18, 2017

I've redone the test such that it fails (on beta & nightly) even without the ICE from #46623.

@arielb1

This comment has been minimized.

Show comment
Hide comment
@arielb1

arielb1 Dec 18, 2017

Contributor

@bors r+

Contributor

arielb1 commented Dec 18, 2017

@bors r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Dec 18, 2017

Contributor

📌 Commit 087f1c2 has been approved by arielb1

Contributor

bors commented Dec 18, 2017

📌 Commit 087f1c2 has been approved by arielb1

@arielb1

This comment has been minimized.

Show comment
Hide comment
@arielb1

arielb1 Dec 18, 2017

Contributor

@bors p=1

This needs to get into beta

Contributor

arielb1 commented Dec 18, 2017

@bors p=1

This needs to get into beta

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Dec 18, 2017

Contributor

⌛️ Testing commit 087f1c2 with merge e7db42f...

Contributor

bors commented Dec 18, 2017

⌛️ Testing commit 087f1c2 with merge e7db42f...

bors added a commit that referenced this pull request Dec 18, 2017

Auto merge of #46808 - eddyb:issue-46769-quick, r=arielb1
rustc: ensure optimized enums have a properly aligned size.

Fixes #46769 by padding the optimized enums wrapping packed data as necessary.

Note that this is not the only way to solve this - on nightly, #46436 makes it easier to fix without adding new padding because of the replacement of `packed` flags with a non-redundant scheme.
But because it can't be backported, the optimal fix will be in a separate nightly-only PR (#46809).
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Dec 18, 2017

Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing e7db42f to master...

Contributor

bors commented Dec 18, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing e7db42f to master...

@bors bors merged commit 087f1c2 into rust-lang:master Dec 18, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@eddyb eddyb deleted the eddyb:issue-46769-quick branch Dec 18, 2017

kennytm added a commit to kennytm/rust that referenced this pull request Dec 21, 2017

Rollup merge of rust-lang#46809 - eddyb:issue-46769-optimal, r=arielb1
 rustc: do not raise the alignment of optimized enums to the niche's alignment.

This is the improved fix for rust-lang#46769 that does not increase the size of any types (see also rust-lang#46808).
@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jan 10, 2018

Member

Looks like we forgot to backport for 1.23.0 (sorry about that!) so removing the beta tags.

Member

alexcrichton commented Jan 10, 2018

Looks like we forgot to backport for 1.23.0 (sorry about that!) so removing the beta tags.

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Jan 10, 2018

Member

@alexcrichton #45225 was backed out from that beta (IIUC) so there wasn't anything to backport.

Member

eddyb commented Jan 10, 2018

@alexcrichton #45225 was backed out from that beta (IIUC) so there wasn't anything to backport.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jan 10, 2018

Member

Ah ok, great!

Member

alexcrichton commented Jan 10, 2018

Ah ok, great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment