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 #[repr(transparent)] to Atomic* types #52149

Merged
merged 1 commit into from
Jul 11, 2018

Conversation

willmo
Copy link
Contributor

@willmo willmo commented Jul 8, 2018

This allows them to be used in #[repr(C)] structs without warnings. Since rust-lang/rfcs#1649 and #35603 they are already documented to have "the same in-memory representation as" their corresponding primitive types. This just makes that explicit.

This was briefly part of #51395, but was controversial and therefore dropped. But it turns out that it's essentially already documented (which I had forgotten).

This allows them to be used in #[repr(C)] structs without warnings. Since rust-lang/rfcs#1649 and rust-lang#35603 they are already documented to have "the same in-memory representation as" their corresponding primitive types. This just makes that explicit.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @shepmaster (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 8, 2018
@cramertj
Copy link
Member

cramertj commented Jul 9, 2018

@bors r+ rollup

@willmo

they are already documented to have "the same in-memory representation as" their corresponding primitive types. This just makes that explicit.

Note that #[repr(transparent)] guarantees more than representation equivalence: it guarantees ABI equivalence, so these values can now be passed via FFI as their inner type.

@bors
Copy link
Contributor

bors commented Jul 9, 2018

📌 Commit e769dec has been approved by cramertj

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 10, 2018
Add #[repr(transparent)] to Atomic* types

This allows them to be used in `#[repr(C)]` structs without warnings. Since rust-lang/rfcs#1649 and rust-lang#35603 they are already documented to have "the same in-memory representation as" their corresponding primitive types. This just makes that explicit.

This was briefly part of rust-lang#51395, but was controversial and therefore dropped. But it turns out that it's essentially already documented (which I had forgotten).
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 10, 2018
Add #[repr(transparent)] to Atomic* types

This allows them to be used in `#[repr(C)]` structs without warnings. Since rust-lang/rfcs#1649 and rust-lang#35603 they are already documented to have "the same in-memory representation as" their corresponding primitive types. This just makes that explicit.

This was briefly part of rust-lang#51395, but was controversial and therefore dropped. But it turns out that it's essentially already documented (which I had forgotten).
bors added a commit that referenced this pull request Jul 10, 2018
Rollup of 7 pull requests

Successful merges:

 - #51612 (NLL: fix E0594 "change to mutable ref" suggestion)
 - #51722 (Updated RELEASES for 1.28.0)
 - #52064 (Clarifying how the alignment of the struct works)
 - #52149 (Add #[repr(transparent)] to Atomic* types)
 - #52151 (Trait impl settings)
 - #52171 (Correct some codegen stats counter inconsistencies)
 - #52195 (rustc: Avoid /tmp/ in graphviz writing)

Failed merges:

 - #52164 (use proper footnote syntax for references)

r? @ghost
@bors bors merged commit e769dec into rust-lang:master Jul 11, 2018
@willmo willmo deleted the transparent-atomics branch July 11, 2018 05:45
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 5, 2018
Added in rust-lang#52149 the discussion in rust-lang#53514 is showing how we may not want to
actually add this attribute to the atomic types. While we continue to
debate rust-lang#53514 this commit reverts the addition of the `transparent` attribute
before it hits stable.
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 5, 2018
Added in rust-lang#52149 the discussion in rust-lang#53514 is showing how we may not want to
actually add this attribute to the atomic types. While we continue to
debate rust-lang#53514 this commit reverts the addition of the `transparent` attribute.
This should be a more conservative route which leaves us the ability to tweak
this in the future but in the meantime allows us to continue discussion as well.
bors added a commit that referenced this pull request Sep 6, 2018
…r=nikomatsakis

[beta]: Remove `#[repr(transparent)]` from atomics

Added in #52149 the discussion in #53514 is showing how we may not want to
actually add this attribute to the atomic types. While we continue to
debate #53514 this commit reverts the addition of the `transparent` attribute
before it hits stable.
kennytm added a commit to kennytm/rust that referenced this pull request Sep 7, 2018
…t-atomics-master, r=sfackler

Remove `#[repr(transparent)]` from atomics

Added in rust-lang#52149 the discussion in rust-lang#53514 is showing how we may not want to
actually add this attribute to the atomic types. While we continue to
debate rust-lang#53514 this commit reverts the addition of the `transparent` attribute.
This should be a more conservative route which leaves us the ability to tweak
this in the future but in the meantime allows us to continue discussion as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants