-
Notifications
You must be signed in to change notification settings - Fork 102
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
Make euclid types repr(C). #176
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me, but be aware of the following comments.
@@ -33,6 +33,7 @@ use std::fmt; | |||
/// another. See the `ScaleFactor` docs for an example. | |||
// Uncomment the derive, and remove the macro call, once heapsize gets | |||
// PhantomData<T> support. | |||
#[repr(C)] | |||
#[derive(RustcDecodable, RustcEncodable)] | |||
pub struct Length<T, Unit>(pub T, PhantomData<Unit>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is safe, even though rustc will complain about it until rust-lang/rust#39462 lands.
Also, note that depending of the size of T, being Length a struct but T an integer/floating point value, passing Length
as an argument instead of T
may break the ABI in 32bit linux, etc, see the last comments in rust-lang/rfcs#1758.
I don't think you're going to pass Length directly through, right?
@@ -14,6 +14,7 @@ macro_rules! define_matrix { | |||
$(pub $field:ident: T,)+ | |||
} | |||
) => ( | |||
#[repr(C)] | |||
$(#[$attr])* | |||
pub struct $name<T, $($phantom),+> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto regarding phantomdata.
@@ -36,6 +36,7 @@ use std::marker::PhantomData; | |||
/// let one_foot: Length<f32, Inch> = Length::new(12.0); | |||
/// let one_foot_in_mm: Length<f32, Mm> = one_foot * mm_per_inch; | |||
/// ``` | |||
#[repr(C)] | |||
#[derive(RustcDecodable, RustcEncodable)] | |||
pub struct ScaleFactor<T, Src, Dst>(pub T, PhantomData<(Src, Dst)>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And ditto again, similarly to Length
.
@bors-servo r+ |
📌 Commit 70ae01e has been approved by |
Make euclid types repr(C). This will facilitate exposing euclid types through ffi. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/176) <!-- Reviewable:end -->
☀️ Test successful - status-travis |
This will facilitate exposing euclid types through ffi.
This change is