Skip to content

Conversation

@pepyakin
Copy link
Contributor

Fixes #1034

@pepyakin
Copy link
Contributor Author

r? @fitzgen

} else {
let ty = helpers::blob(Layout::new(0, layout.align));
fields.push(quote! {
pub __bindgen_align: #ty ,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only work to increase alignment, right? So what if we are decreasing alignment (but not all the way to 1)? I guess that requires #pragma packed or __attribute__((aligned(..))) which we don't fully support yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. If one would try to decrease alignment it would lead to layout test failure... just as it is now.

#( #attributes )*
pub struct #canonical_ident
}
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole chunk is just code motion right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

@fitzgen
Copy link
Member

fitzgen commented Oct 11, 2017

Thanks as always, @pepyakin !

@bors-servo r+

@bors-servo
Copy link

📌 Commit 746c743 has been approved by fitzgen

@bors-servo
Copy link

⌛ Testing commit 746c743 with merge c88ddab...

bors-servo pushed a commit that referenced this pull request Oct 11, 2017
Use `repr(packed)` If struct requires explicit alignment of 1.

Fixes #1034
@bors-servo
Copy link

☀️ Test successful - status-travis
Approved by: fitzgen
Pushing c88ddab to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants