-
Notifications
You must be signed in to change notification settings - Fork 744
codegen: Improve the assertion message of failing layout tests. #487
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
Conversation
Actually, @flier, wanna review this? :) |
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.
Didn't go through the expectations' diff, but this looks great to me :)
r=me (travis CI matrix changes ok as follow up, or split out from this PR)
@@ -2745,7 +2745,7 @@ mod utils { | |||
} | |||
|
|||
pub fn fnsig_return_ty(ctx: &BindgenContext, | |||
sig: &super::FunctionSig) | |||
sig: &FunctionSig) |
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.
If we are going to support older rustc, we should add the oldest supported version to our travis ci matrix.
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.
This is only temporary until Gecko updates is rustc version anyway, should be <1week
@bors-servo r=fitzgen |
📌 Commit 193a103 has been approved by |
assert_eq!($size_of_expr, $size, | ||
concat!("Size of template specialization: ", stringify!($ident))); | ||
assert_eq!($align_of_expr, $align, | ||
concat!("Alignment of template specialization: ", stringify!($ident))); | ||
}) |
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.
The patch is good to me, something else observed.
Do we need check the field offset for the template specialization?
It seems be different to VTable case, should have potential layout issue. Anyway, I haven't user cases in wild
☀️ Test successful - status-travis |
r? @fitzgen
cc @flier