-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
improve error message shown for unsafe operations #52207
Conversation
…ned behavior could arise Inspired by @gnzlbg at rust-lang#46043 (comment)
(rust_highfive has picked a reviewer for you, use r? to override) |
r? @estebank |
= note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior | ||
|
||
error: aborting due to 2 previous errors | ||
|
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 is really nice, thank you.
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.
And it took me only three months since you suggested it...^^
@bors r+ rollup |
📌 Commit f511c5e has been approved by |
"assignment to non-`Copy` union field") | ||
"assignment to non-`Copy` union field", | ||
"the previous content of the field may be dropped, which \ | ||
cause undefined behavior if the field was not properly \ |
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.
Typo: "which cause"
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 must be blind, what is the typo there?
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.
Oh, it must be "which causes", of course. Sorry!
I should also add that I am not yet sure why "assignment to non- |
I verified from the RFC that this is exactly what happens. (There is a more elaborate scheme being proposed where fields are only sometimes dropped, but right now it seems to always drop.) |
Fixed typo. |
@bors r=estebank |
@RalfJung: 🔑 Insufficient privileges: Not in reviewers |
@estebank looks like you have to r+ yourself. |
@bors r+ |
📌 Commit f68323b has been approved by |
improve error message shown for unsafe operations Add a short explanation saying why undefined behavior could arise. In particular, the error many people got for "creating a pointer to a packed field requires unsafe block" was not worded great -- it lead to people just adding the unsafe block without considering if what they are doing follows the rules. I am not sure if a "note" is the right thing, but that was the easiest thing to add... Inspired by @gnzlbg at rust-lang#46043 (comment)
Rollup of 14 pull requests Successful merges: - #51614 (Correct suggestion for println) - #51952 ( hygiene: Decouple transparencies from expansion IDs) - #52193 (step_by: leave time of item skip unspecified) - #52207 (improve error message shown for unsafe operations) - #52223 (Deny bare trait objects in in src/liballoc) - #52224 (Deny bare trait objects in in src/libsyntax) - #52239 (Remove sync::Once::call_once 'static bound) - #52247 (Deny bare trait objects in in src/librustc) - #52248 (Deny bare trait objects in in src/librustc_allocator) - #52252 (Deny bare trait objects in in src/librustc_codegen_llvm) - #52253 (Deny bare trait objects in in src/librustc_data_structures) - #52254 (Deny bare trait objects in in src/librustc_metadata) - #52261 (Deny bare trait objects in in src/libpanic_unwind) - #52265 (Deny bare trait objects in in src/librustc_codegen_utils) Failed merges: r? @ghost
Add a short explanation saying why undefined behavior could arise. In particular, the error many people got for "creating a pointer to a packed field requires unsafe block" was not worded great -- it lead to people just adding the unsafe block without considering if what they are doing follows the rules.
I am not sure if a "note" is the right thing, but that was the easiest thing to add...
Inspired by @gnzlbg at #46043 (comment)