-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Fix projections when parent capture is by-ref but child capture is by-value in the ByMoveBody
pass
#129101
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
r? lcnr lol idk who else to request review from sorry |
@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. |
@bors r+ rollup |
…-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
…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
…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
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
forget about it, homu. the loop's over. @bors r- |
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: &Ty
by-ref, resulting in an upvar of&&Ty
. The child coroutine capturesx
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
isCopy
, since we'd instead capturex
by-ref in the child coroutine.Fixes #129074