-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Check if call return type is visibly uninhabited when building MIR #93313
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 |
---|---|---|
|
@@ -10,14 +10,11 @@ LL | unsafe { std::mem::transmute(()) } | |
LL | const FOO: [Empty; 3] = [foo(); 3]; | ||
| ----- inside `FOO` at $DIR/validate_uninhabited_zsts.rs:13:26 | ||
|
||
error[E0080]: it is undefined behavior to use this value | ||
--> $DIR/validate_uninhabited_zsts.rs:16:1 | ||
error[E0080]: evaluation of constant value failed | ||
--> $DIR/validate_uninhabited_zsts.rs:16:35 | ||
| | ||
LL | const BAR: [Empty; 3] = [unsafe { std::mem::transmute(()) }; 3]; | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at [0]: encountered a value of uninhabited type Empty | ||
| | ||
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. This is a somewhat unfortunate reduction in test coverage for validity (we now error much earlier during evaluation)... ideally this would be adjusted to transmute to a type that is not 'visibly' uninhabited so that we still test the same code paths as before. 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. I did the same in a Miri test so that type definition can probably just be copied. |
||
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. | ||
= note: the raw bytes of the constant (size: 0, align: 1) {} | ||
| ^^^^^^^^^^^^^^^^^^^^^^^ transmuting to uninhabited type | ||
|
||
warning: the type `!` does not permit zero-initialization | ||
--> $DIR/validate_uninhabited_zsts.rs:4:14 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
// edition:2021 | ||
// run-pass | ||
// compile-flags: -Zdrop-tracking | ||
|
||
#![feature(never_type)] | ||
|
||
|
@@ -32,7 +33,7 @@ fn never() -> Never { | |
} | ||
|
||
async fn includes_never(crash: bool, x: u32) -> u32 { | ||
let mut result = async { x * x }.await; | ||
let result = async { x * x }.await; | ||
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. This behavior change seems correct, but I wonder if we need to be concerned from a compatibility perspective? 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. To be clear: Wesley's assumption is that the unused And Wesley's question is: Do we need to worry about this? Considering e.g. code that might have (My personal answer is that lints are allowed to change their behavior in this manner. I'm currently double-checking/asking around for examples from Rust's past.) 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. 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. The lint side of things seems fine to me from compatibility perspective. It is The fact that we would now accept more programs seems more significant, since 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. It also seems surprising that this 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. I was talking about this with @scottmcm a little while back and he found this example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=26220b66188903b1dd8f3325960f75af That version compiles even though |
||
if !crash { | ||
return result; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// Verifies that MIR building for a call expression respects | ||
// privacy when checking if a call return type is uninhabited. | ||
|
||
pub mod widget { | ||
enum Unimplemented {} | ||
pub struct Widget(Unimplemented); | ||
|
||
impl Widget { | ||
pub fn new() -> Widget { | ||
todo!(); | ||
} | ||
} | ||
|
||
pub fn f() { | ||
let x: &mut u32; | ||
Widget::new(); | ||
// Ok. Widget type returned from new is known to be uninhabited | ||
// and the following code is considered unreachable. | ||
*x = 1; | ||
} | ||
} | ||
|
||
fn main() { | ||
let y: &mut u32; | ||
widget::Widget::new(); | ||
// Error. Widget type is not known to be uninhabited here, | ||
// so the following code is considered reachable. | ||
*y = 2; //~ ERROR use of possibly-uninitialized variable | ||
} |
Uh oh!
There was an error while loading. Please reload this page.