[Drop Tracking] Allow uninhabited types in generator interiors #94752
Conversation
Drop tracking in the generator interior type checking pass would count all values in unreachable code as dropped (e.g. code after a call to a function with an uninhabited return type), which would lead to those values not being included in the witness type. This resulted in the type checker being more precise than the corresponding sanity check in the MIR transform. This patch changes the check in the MIR code to match the results of typeck by skipping uninhabited types. It also reduces the test case that was added earlier.
rustbot
added
the
T-compiler
|
r? @wesleywiser (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
eholk
changed the title
|
Types included in a generator interior by an "unreachable" code could be arbitrary. I suspect that the following test case still fails, hence my different suggestion in #93313 (comment). enum Never {}
fn f() -> Never { todo!() }
fn main() {
let _ = async {
let a = String::new();
let b = f();
async {}.await;
drop(a);
drop(b);
};
} |
|
Ah, good point. Your test case does indeed fail. I just pushed a new patch that instead removes the |
eholk
changed the title
eholk
changed the title
|
And I changed it again. I realized we can keep the optimization on the typeck side if we make it so on the MIR side we can skip the sanity check if there are any uninhabited types in scope, since that means the code is unreachable. |
| // First check if any of the locals are an uninhabited type. If so, we don't need to do the | ||
| // rest of this check because this code unreachable. | ||
| let any_uninhabited = body.local_decls.iter_enumerated().any(|(_, decl)| { |
There was a problem hiding this comment.
Since this validates a complete generator, we don't really know which parts of it are unreachable if any.
If we want to optimize out code following call with uninhabited type as unreachable, we should do that by transforming the MIR and ensuring those locals are not saved in the first place, while keeping the validation in place to ensure that typeck and MIR are in sync.
|
I believe this is blocked on #93313 which is currently undergoing a lang team fcp. Marking as such. |
wesleywiser
added
the
S-blocked
|
☔ The latest upstream changes (presumably #96253) made this pull request unmergeable. Please resolve the merge conflicts. |
|
This is not needed now that #93313 has merged, so I will go ahead and close it. |
apiraino
removed
S-waiting-on-review
Drop tracking in the generator interior type checking pass would count all values in unreachable code as dropped (e.g. code after a call to a function with an uninhabited return type), which would lead to those values not being included in the witness type. This resulted in the type checker being more precise than the corresponding sanity check in the MIR transform.
This PR updates the sanity check on the MIR side to interpret any uninhabited types in the generator witness type as meaning the whole section of code is unreachable, so there is no restriction on what types are allowed in the generator.