-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Handle partial initialization correctly in generators #63616
Copy link
Copy link
Closed
Labels
A-MIRArea: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.htmlArea: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.htmlA-async-awaitArea: Async & AwaitArea: Async & AwaitA-coroutinesArea: CoroutinesArea: CoroutinesAsyncAwait-TriagedAsync-await issues that have been triaged during a working group meeting.Async-await issues that have been triaged during a working group meeting.C-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Metadata
Metadata
Assignees
Labels
A-MIRArea: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.htmlArea: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.htmlA-async-awaitArea: Async & AwaitArea: Async & AwaitA-coroutinesArea: CoroutinesArea: CoroutinesAsyncAwait-TriagedAsync-await issues that have been triaged during a working group meeting.Async-await issues that have been triaged during a working group meeting.C-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Type
Fields
Give feedbackNo fields configured for issues without a type.
Problem: consider something like the following,
This can result in a local saved in our generator of type
((), !), which is partially initialized across the yield. In the layout code we claim that fields are all fully initialized across yields. (This is still a problem even if the second field is not of type!, but is particularly egregious in that case.)Today this doesn't result in any miscompilations. MIR generation is quite conservative, and never seems to do partial initialization of aggregates across blocks. That might change in the future, and the layouts we create for generators do not take this into account.
The simplest correct thing we can do is to wrap all saved Generator locals in
MaybeUninit. This means we need to project through the union in the generator MIR transform, which makes it different from the way we wrap withMaybeUninit"internally" today, to avoid propagating uninhabitedness (see #63035).The only downside of wrapping everything is that we lose information that might be useful for optimizations and in miri. We can do better than this by only wrapping fields of an aggregate type, or maybe even restricting to fields which contain a field projection in the generator code.
#63230 was a naive attempt at avoiding issues like this, but avoiding partial initialization in surface Rust is not enough to prevent it in MIR.