Skip to content

/rust-bindgen

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

repr(C) on enums #1183

Merged
merged 2 commits into from Dec 14, 2017

Conversation

Reviewers
No reviews
Assignees

@emilio emilio

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@fitzgen @highfive @emilio @bors-servo @pepyakin
@fitzgen
Copy link
Member

fitzgen commented Dec 9, 2017

r? @pepyakin or @emilio

@highfive highfive assigned pepyakin Dec 9, 2017

@highfive

This comment has been minimized.

Sign in to view
Copy link
Collaborator

highfive commented Dec 9, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@highfive highfive added the S-awaiting-review label Dec 9, 2017

@fitzgen fitzgen force-pushed the fitzgen:repr-c-on-enums branch 2 times, most recently from 978f221 to d43faa4 Dec 12, 2017

@fitzgen
Always add `repr(C)` to rustified enums 
If we don't, then eddyb's recent layout optimizations will do unexpected things
to them.

We also need to handle empty `enum`s without variants. When `repr(C)` is on a
Rust `enum`, it cannot be empty, so we need to add a dummy.
Loading status checks…
43c43ba

@fitzgen fitzgen force-pushed the fitzgen:repr-c-on-enums branch from d43faa4 to 43c43ba Dec 12, 2017

@fitzgen

This comment has been minimized.

Sign in to view
Copy link
Member Author

fitzgen commented Dec 12, 2017

@emilio fixed so that both repr(C) and repr(i32) (or whatever) are added. Also fixed other libclang versions test expectations.
@emilio

This comment has been minimized.

Sign in to view
Copy link
Collaborator

emilio commented Dec 14, 2017

@bors-servo r+
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Dec 14, 2017

📌 Commit 43c43ba has been approved by emilio

@highfive highfive assigned emilio and unassigned pepyakin Dec 14, 2017

@highfive highfive removed the S-awaiting-review label Dec 14, 2017

@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Dec 14, 2017

⌛️ Testing commit 43c43ba with merge 84f8e18...

@highfive highfive added the S-awaiting-merge label Dec 14, 2017

bors-servo added a commit that referenced this pull request Dec 14, 2017

@bors-servo
Auto merge of #1183 - fitzgen:repr-c-on-enums, r=emilio 
repr(C) on enums

r? @pepyakin or @emilio
Loading status checks…
84f8e18
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Dec 14, 2017

☀️ Test successful - status-travis
Approved by: emilio
Pushing 84f8e18 to master...

@bors-servo bors-servo merged commit 43c43ba into rust-lang:master Dec 14, 2017
2 checks passed

2 checks passed

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

@highfive highfive removed the S-awaiting-merge label Dec 14, 2017

@fitzgen fitzgen deleted the fitzgen:repr-c-on-enums branch Dec 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.