-
Notifications
You must be signed in to change notification settings - Fork 13.5k
const validation: properly ignore zero-sized UnsafeCell #143046
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
base: master
Are you sure you want to change the base?
Conversation
let zst = self | ||
.ecx | ||
.size_and_align_of(&val.meta(), &val.layout)? | ||
.is_some_and(|(s, _a)| s.bytes() == 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.
this seems very footgunny, why do is_zst
and size_and_align_of(&val.meta(), &val.layout).is_some_and(|(s, _a)| s.bytes() == 0)
differ. When is using is_zst
actually correct?
Okay, this is because we're not checking whether the type itself is a ZST, but that the value is a zero-sized. So we don't use is_zst
to also handle slices and trait objects. Can you add this as a comment to the code in visit_value
?
after that r=me
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.
When is using is_zst actually correct?
Basically this is must_be_zst
, but it can miss types that are only dynamically zero-sized.
d0fce3e
to
59e66a6
Compare
The Miri subtree was changed cc @rust-lang/miri Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
I also added a 2nd commit making the size_and_align_of APIs in the interpreter a bit nicer. |
59e66a6
to
7de39f5
Compare
@bors r=lcnr,oli-obk |
Fixes #142948
r? @oli-obk