-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Implement more extra_traits #1271
Conversation
r? @gnzlbg (rust_highfive has picked a reviewer for you, use r? to override) |
Cool! Thanks! Ping me when CI is green! |
26d9518
to
3f55d23
Compare
So 1.24 is the cutoff for using |
5f8e5b7
to
d54423f
Compare
The problem is that, if creating a reference to a packed struct field creates an unaligned reference, then the behavior is undefined. That's the case irrespectively of whether one uses an unsafe block post Rust 1.24, or no unsafe block in older Rust versions. |
You could just add an attribute to those methods to silence the unused unsafe warning (e.g. " EDIT: you also cannot create an unaligned reference, and then cast it to a pointer, and then check whether it is aligned or not, because the UB happens before the check . |
Alternatively, one can take a reference to the struct, cast it to a pointer, and use pointer offsets to read from the struct fields. One'd need to compute the offsets manually, but the structs are |
Yes, that is the best approach IMO. You need a reference to call |
0d84124
to
d5035cd
Compare
f13fe7d
to
d795b07
Compare
Remaining failures are not due to this code change and for targets that are allowed to fail. I think this is all good now. |
@bors: r+ |
📌 Commit d795b07 has been approved by |
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.
LGTM!
Implement more extra_traits Finishing the implementation of rust-lang/rust#57715 now that all platforms are tested in CI. I plan to expand this CI to all platforms that need this treatment (solarish, empscripten, freebsd, and netbsd), but thought I'd get the part I've already started running in CI since I can't test this changes locally.
☀️ Test successful - checks-cirrus, checks-travis, status-appveyor |
Finishing the implementation of rust-lang/rust#57715 now that all platforms are tested in CI. I plan to expand this CI to all platforms that need this treatment (solarish, empscripten, freebsd, and netbsd), but thought I'd get the part I've already started running in CI since I can't test this changes locally.