-
Notifications
You must be signed in to change notification settings - Fork 744
codegen: Don't assume unsized structs have address. #552
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
src/codegen/mod.rs
Outdated
fields.push(field); | ||
let has_address = match layout { | ||
Some(l) => l.size != 0, | ||
None => true, |
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.
Little shorter variant: layout.map_or(true, |l| l.size != 0)
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.
+1 for the shorter variant.
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.
Thanks! :)
src/codegen/mod.rs
Outdated
fields.push(field); | ||
let has_address = match layout { | ||
Some(l) => l.size != 0, | ||
None => true, |
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.
+1 for the shorter variant.
@@ -0,0 +1 @@ | |||
struct Foo {}; |
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.
I know we have tests for this intermingled here and there elsewhere, but it would be nice to have a C++ companion for this test that has the same contents but with a .hpp
extension.
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.
Done :)
Per C semantics, they may not.
@bors-servo r=fitzgen |
📌 Commit 5982cd3 has been approved by |
codegen: Don't assume unsized structs have address. Per C semantics, they may not. r? @fitzgen
☀️ Test successful - status-travis |
Per C semantics, they may not.
r? @fitzgen