-
Notifications
You must be signed in to change notification settings - Fork 691
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
Compute sizedness with a fixed-point analysis #1102
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.
looks great!
src/ir/analysis/sizedness.rs
Outdated
} | ||
|
||
TypeKind::ObjCInterface(..) => { | ||
trace!(" I guess obj-c interfaces are not zero-sized?"); |
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.
FWIW, as far as I know, every obj-c "interface" have an isa
pointer.
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.
Cool, I'll note this. FWIW, I was just copying existing behavior.
trace!(" comp considers its own fields and bases"); | ||
|
||
if !info.fields().is_empty() { | ||
return self.insert(id, SizednessResult::NonZeroSized); |
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.
But what if all fields are zero sized 🤔 ?
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.
They can't be, because of the every-object-must-be-addressable rules. Only base members can truly be zero sized, because its the "same" object.
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.
That is, with
struct Empty {};
struct Inherits : public Empty {
int x;
};
struct Contains {
Empty empty;
int x;
};
sizeof(Contains)
is one greater than sizeof(Inherits)
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.
yeah, makes sense. Thank you!
Thanks for the review @pepyakin ! |
This fixes a couple bugs where we weren't properly adding an `_address` byte. It also helps pave the way for computing implicit fields (such as padding, `_address`, vtable pointers, etc) in its own pass, before codegen. Fixes rust-lang#768
I added that example, and another for arrays of zero-sized types, as tests. Thanks again for review @pepyakin ! @bors-servo r=pepyakin |
📌 Commit d018f42 has been approved by |
Added one more test for arrays of zero elements and flexible arrays also. @bors-servo r=pepyakin |
📌 Commit fd7afb6 has been approved by |
☀️ Test successful - status-travis |
This fixes a couple bugs where we weren't properly adding an
_address
byte. It also helps pave the way for computing implicit fields (such as padding,_address
, vtable pointers, etc) in its own pass, before codegen.Fixes #768
r? @pepyakin