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.

Sign up for GitHub

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

Jump to bottom

Quantity should be repr(transparent) #145

Closed
gnzlbg opened this issue Jun 13, 2019 · 1 comment · Fixed by #146
Closed

Quantity should be repr(transparent) #145

gnzlbg opened this issue Jun 13, 2019 · 1 comment · Fixed by #146

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 13, 2019

Is there a reason why this is not the case ? Without repr(transparent) Quantity has a struct ABI, instead of the ABI of T (scalar float or scalar integer). This means that one cannot use Quantity in C FFI, and it also might mean that the compiler might generate sub-optimal code for those situations in which the struct ABI is worse than the float or integer one.
gnzlbg added a commit to gnzlbg/uom that referenced this issue Jun 13, 2019
@gnzlbg
Make Quantity repr(transparent)
d005164 
Closes iliekturtles#145 .
@gnzlbg gnzlbg mentioned this issue Jun 13, 2019
Merged
@iliekturtles
Copy link
Owner

Looks like work in rustc prior to 1.28.0, when #repr[transparent] was stabilized, fixed this issue (rust-lang/rust#38269) in pure Rust and I never added #repr[transparent] for FFI once it stabilized. I'll bump uom's minimum rustc version and then merge your PR. Thanks!

iliekturtles added a commit that referenced this issue Jun 13, 2019
@iliekturtles
Increase minimum supported rustc version to 1.28.0.
8c92538 
Changed in anticipation of adding `#[repr(transparent)]` in #145.
Zero-cost codegen note regarding `rustc` 1.25.0 is now no longer
relevant.
@iliekturtles iliekturtles mentioned this issue Jun 13, 2019
Merged
iliekturtles added a commit that referenced this issue Jun 15, 2019
@iliekturtles
Increase minimum supported rustc version to 1.28.0.
d36ba5d 
Changed in anticipation of adding `#[repr(transparent)]` in #145.
Zero-cost codegen note regarding `rustc` 1.25.0 is now no longer
relevant.
iliekturtles added a commit that referenced this issue Jun 15, 2019
@iliekturtles
Increase minimum supported rustc version to 1.28.0.
711e9e2 
Changed in anticipation of adding `#[repr(transparent)]` in #145.
Zero-cost codegen note regarding `rustc` 1.25.0 is now no longer
relevant.
iliekturtles added a commit that referenced this issue Jun 15, 2019
@iliekturtles
Increase minimum supported rustc version to 1.28.0.
8eb92bb 
Changed in anticipation of adding `#[repr(transparent)]` in #145.
Zero-cost codegen note regarding `rustc` 1.25.0 is now no longer
relevant.
iliekturtles added a commit that referenced this issue Jun 16, 2019
@iliekturtles
Increase minimum supported rustc version to 1.28.0.
6001078 
Changed in anticipation of adding `#[repr(transparent)]` in #145.
Zero-cost codegen note regarding `rustc` 1.25.0 is now no longer
relevant.
iliekturtles added a commit that referenced this issue Jun 16, 2019
@iliekturtles
Increase minimum supported rustc version to 1.28.0.
ee03ca6 
Changed in anticipation of adding `#[repr(transparent)]` in #145.
Zero-cost codegen note regarding `rustc` 1.25.0 is now no longer
relevant.
iliekturtles added a commit that referenced this issue Jun 17, 2019
@iliekturtles
Increase minimum supported rustc version to 1.28.0.
cd9223d 
Changed in anticipation of adding `#[repr(transparent)]` in #145.
Zero-cost codegen note regarding `rustc` 1.25.0 is now no longer
relevant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make Quantity repr(transparent) gnzlbg/uom
2 participants
@gnzlbg @iliekturtles