improve error message shown for unsafe operations#52207
Merged
bors merged 2 commits intorust-lang:masterfrom Jul 11, 2018
Merged
improve error message shown for unsafe operations#52207bors merged 2 commits intorust-lang:masterfrom
bors merged 2 commits intorust-lang:masterfrom
Conversation
…ned behavior could arise Inspired by @gnzlbg at rust-lang#46043 (comment)
Contributor
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Member
|
r? @estebank |
gnzlbg
reviewed
Jul 10, 2018
| = 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 | ||
|
|
Contributor
There was a problem hiding this comment.
This is really nice, thank you.
Member
Author
There was a problem hiding this comment.
And it took me only three months since you suggested it...^^
Contributor
|
@bors r+ rollup |
Collaborator
|
📌 Commit f511c5e has been approved by |
bors
added
S-waiting-on-bors
| "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 \ |
Member
Author
There was a problem hiding this comment.
I must be blind, what is the typo there?
Member
Author
There was a problem hiding this comment.
Oh, it must be "which causes", of course. Sorry!
Contributor
bors
added
S-waiting-on-author
Member
Author
|
I should also add that I am not yet sure why "assignment to non- |
Member
Author
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.) |
Member
Author
|
Fixed typo. |
Member
Author
|
@bors r=estebank |
Collaborator
|
@RalfJung: 🔑 Insufficient privileges: Not in reviewers |
Member
Author
|
@estebank looks like you have to r+ yourself. |
Contributor
|
@bors r+ |
Collaborator
|
📌 Commit f68323b has been approved by |
bors
added
S-waiting-on-bors
Mark-Simulacrum
added a commit
to Mark-Simulacrum/rust
that referenced
this pull request
Jul 11, 2018
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)
bors
added a commit
that referenced
this pull request
Jul 11, 2018
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)