Fix projections when parent capture is by-ref but child capture is by-value in the ByMoveBody pass#129101
Merged
bors merged 1 commit intorust-lang:masterfrom Aug 15, 2024
Merged
Conversation
Collaborator
Collaborator
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
Contributor
Author
|
r? lcnr lol idk who else to request review from sorry |
Contributor
|
@bors r+ rollup This doesn't affect the While I don't fully understand the surrounding context, the change itself doesn't seem to make anything worse and I trust both @compiler-errors and MIR validation to catch any issues. |
Contributor
|
@bors r+ rollup |
Collaborator
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
Aug 15, 2024
…-ref, r=lcnr Fix projections when parent capture is by-ref but child capture is by-value in the `ByMoveBody` pass This fixes a somewhat strange bug where we build the incorrect MIR in rust-lang#129074. This one is weird, but I don't expect it to actually matter in practice since it almost certainly results in a move error in borrowck. However, let's not ICE. Given the code: ``` #![feature(async_closure)] // NOT copy. struct Ty; fn hello(x: &Ty) { let c = async || { *x; //~^ ERROR cannot move out of `*x` which is behind a shared reference }; } fn main() {} ``` The parent coroutine-closure captures `x: &Ty` by-ref, resulting in an upvar of `&&Ty`. The child coroutine captures `x` by-value, resulting in an upvar of `&Ty`. When constructing the by-move body for the coroutine-closure, we weren't applying an additional deref projection to convert the parent capture into the child capture, resulting in an type error in assignment, which is a validation ICE. As I said above, this only occurs (AFAICT) in code that eventually results in an error, because it is only triggered by HIR that attempts to move a non-copy value out of a ref. This doesn't occur if `Ty` is `Copy`, since we'd instead capture `x` by-ref in the child coroutine. Fixes rust-lang#129074
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 15, 2024
…llaumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#128348 (Unconditionally allow shadow call-stack sanitizer for AArch64) - rust-lang#128922 (rust-analyzer: use in-tree `pattern_analysis` crate) - rust-lang#128935 (More work on `zstd` compression) - rust-lang#129072 (Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return) - rust-lang#129101 (Fix projections when parent capture is by-ref but child capture is by-value in the `ByMoveBody` pass) - rust-lang#129106 (Remove redundant type ops: `Eq`/`Subtype`) - rust-lang#129122 (Remove duplicated `Rustdoc::output` method from `run-make-support` lib) - rust-lang#129124 (rustdoc-json: Use FxHashMap from rustdoc_json_types) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 15, 2024
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#128348 (Unconditionally allow shadow call-stack sanitizer for AArch64) - rust-lang#129065 (Use `impl PartialEq<TokenKind> for Token` more.) - rust-lang#129072 (Infer async closure args from `Fn` bound even if there is no corresponding `Future` bound on return) - rust-lang#129096 (Print more verbose error for commands that capture output) - rust-lang#129101 (Fix projections when parent capture is by-ref but child capture is by-value in the `ByMoveBody` pass) - rust-lang#129106 (Remove redundant type ops: `Eq`/`Subtype`) - rust-lang#129122 (Remove duplicated `Rustdoc::output` method from `run-make-support` lib) - rust-lang#129124 (rustdoc-json: Use FxHashMap from rustdoc_json_types) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 15, 2024
Rollup merge of rust-lang#129101 - compiler-errors:deref-on-parent-by-ref, r=lcnr Fix projections when parent capture is by-ref but child capture is by-value in the `ByMoveBody` pass This fixes a somewhat strange bug where we build the incorrect MIR in rust-lang#129074. This one is weird, but I don't expect it to actually matter in practice since it almost certainly results in a move error in borrowck. However, let's not ICE. Given the code: ``` #![feature(async_closure)] // NOT copy. struct Ty; fn hello(x: &Ty) { let c = async || { *x; //~^ ERROR cannot move out of `*x` which is behind a shared reference }; } fn main() {} ``` The parent coroutine-closure captures `x: &Ty` by-ref, resulting in an upvar of `&&Ty`. The child coroutine captures `x` by-value, resulting in an upvar of `&Ty`. When constructing the by-move body for the coroutine-closure, we weren't applying an additional deref projection to convert the parent capture into the child capture, resulting in an type error in assignment, which is a validation ICE. As I said above, this only occurs (AFAICT) in code that eventually results in an error, because it is only triggered by HIR that attempts to move a non-copy value out of a ref. This doesn't occur if `Ty` is `Copy`, since we'd instead capture `x` by-ref in the child coroutine. Fixes rust-lang#129074
Member
|
forget about it, homu. the loop's over. @bors r- |
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
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.
This fixes a somewhat strange bug where we build the incorrect MIR in #129074. This one is weird, but I don't expect it to actually matter in practice since it almost certainly results in a move error in borrowck. However, let's not ICE.
Given the code:
The parent coroutine-closure captures
x: &Tyby-ref, resulting in an upvar of&&Ty. The child coroutine capturesxby-value, resulting in an upvar of&Ty. When constructing the by-move body for the coroutine-closure, we weren't applying an additional deref projection to convert the parent capture into the child capture, resulting in an type error in assignment, which is a validation ICE.As I said above, this only occurs (AFAICT) in code that eventually results in an error, because it is only triggered by HIR that attempts to move a non-copy value out of a ref. This doesn't occur if
TyisCopy, since we'd instead capturexby-ref in the child coroutine.Fixes #129074