-
Notifications
You must be signed in to change notification settings - Fork 14.1k
Reduce confusing unreachable_code lints
#149044
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| //! Test that a diverging function as the final expression in a block does not | ||
| //! raise an 'unreachable code' lint. | ||
|
|
||
| //@ check-pass | ||
| #![deny(unreachable_code)] | ||
|
|
||
| enum Never {} | ||
|
|
||
| fn make_never() -> Never { | ||
| loop {} | ||
| } | ||
|
|
||
| fn func() { | ||
| make_never(); | ||
| } | ||
|
|
||
| fn block() { | ||
| { | ||
| make_never(); | ||
| } | ||
| } | ||
|
|
||
| fn branchy() { | ||
| if false { | ||
| make_never(); | ||
| } else { | ||
| make_never(); | ||
| } | ||
| } | ||
|
|
||
| fn main() { | ||
| func(); | ||
| block(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,8 +6,9 @@ enum Void {} | |
| fn with_void() { | ||
| if false { | ||
| unsafe { | ||
| //~^ ERROR unreachable expression | ||
| std::mem::uninitialized::<Void>(); | ||
| println!(); | ||
| //~^ ERROR unreachable expression | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where does the error point on the unchanged test? Do you mind adding another
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No error is raised within the block without the additional println (similarly to
Sorry, can you add an example? I'm not quite sure what this should look like |
||
| } | ||
|
|
||
|
|
@@ -20,8 +21,9 @@ fn infallible() -> std::convert::Infallible { | |
|
|
||
| fn with_infallible() { | ||
| if false { | ||
| //~^ ERROR unreachable expression | ||
| infallible(); | ||
| println!() | ||
| //~^ ERROR unreachable expression | ||
| } | ||
|
|
||
| println!() | ||
|
|
||
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.
Do you mind adding a comment explaining why we return
Nonehere?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.
// No user code in this bb, and our goto target may be reachable via other pathsThere may be other cases where we shouldn't lint and/or should label with something other than
"expression"but this was sufficient for the motivating examplesThere 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'm skeptical about the goto. What about some kind of
if a { panic!() } else { unreachable!() }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.
Current (nightly) error
Two errors, each highlighting the if and else blocks respectivelyWith the current PR no error is emitted at all.
With the `Goto` arm removed
Two errors, both highlighting the entire if/else
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.
r=me with a test illustrating this.
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.
Added a test for the current behaviour.