about summary refs log tree commit diff
diff options
context:
space:
mode:
authorGuillaume Gomez <guillaume1.gomez@gmail.com>2024-04-09 13:39:23 +0200
committerGitHub <noreply@github.com>2024-04-09 13:39:23 +0200
commitcfe1faa75df166f79482b9553253c104712aa9bd (patch)
tree94df66f457fdf75005d4c81dd4bc548fc684f8ab
parentb3f40e3448460a7dc5321948a049f43f23aada24 (diff)
parent54a93ab11e183e25ba9addc50655cf16e2614329 (diff)
downloadrust-cfe1faa75df166f79482b9553253c104712aa9bd.tar.gz
rust-cfe1faa75df166f79482b9553253c104712aa9bd.zip
Rollup merge of #123658 - compiler-errors:stop-assuming, r=oli-obk
Stop making any assumption about the projections applied to the upvars in the `ByMoveBody` pass

So it turns out that because of subtle optimizations like [`truncate_capture_for_optimization`](https://github.com/rust-lang/rust/blob/ab5bda1aa70f707014e2e691e43bc37a8819252a/compiler/rustc_hir_typeck/src/upvar.rs#L2351), we simply cannot make any assumptions about the shape of the projections applied to the upvar locals in a coroutine body.

So stop doing that -- the code is resilient to such projections, so the assertion really existed only to "protect against the unknown".

r? oli-obk
Fixes #123650
-rw-r--r--compiler/rustc_mir_transform/src/coroutine/by_move_body.rs36
-rw-r--r--tests/ui/async-await/async-closures/truncated-fields-when-imm.rs17
2 files changed, 34 insertions, 19 deletions
diff --git a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs
index 320d8fd3977..d6ec7d64926 100644
--- a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs
+++ b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs
@@ -281,14 +281,14 @@ impl<'tcx> MutVisitor<'tcx> for MakeByMoveBody<'tcx> {
         if place.local == ty::CAPTURE_STRUCT_LOCAL
             && let Some((&mir::ProjectionElem::Field(idx, _), projection)) =
                 place.projection.split_first()
-            && let Some(&(remapped_idx, remapped_ty, needs_deref, additional_projections)) =
+            && let Some(&(remapped_idx, remapped_ty, needs_deref, bridging_projections)) =
                 self.field_remapping.get(&idx)
         {
             // As noted before, if the parent closure captures a field by value, and
             // the child captures a field by ref, then for the by-move body we're
             // generating, we also are taking that field by value. Peel off a deref,
-            // since a layer of reffing has now become redundant.
-            let final_deref = if needs_deref {
+            // since a layer of ref'ing has now become redundant.
+            let final_projections = if needs_deref {
                 let Some((mir::ProjectionElem::Deref, projection)) = projection.split_first()
                 else {
                     bug!(
@@ -302,20 +302,18 @@ impl<'tcx> MutVisitor<'tcx> for MakeByMoveBody<'tcx> {
                 projection
             };
 
-            // The only thing that should be left is a deref, if the parent captured
-            // an upvar by-ref.
-            std::assert_matches::assert_matches!(final_deref, [] | [mir::ProjectionElem::Deref]);
-
-            // For all of the additional projections that come out of precise capturing,
-            // re-apply these projections.
-            let additional_projections =
-                additional_projections.iter().map(|elem| match elem.kind {
-                    ProjectionKind::Deref => mir::ProjectionElem::Deref,
-                    ProjectionKind::Field(idx, VariantIdx::ZERO) => {
-                        mir::ProjectionElem::Field(idx, elem.ty)
-                    }
-                    _ => unreachable!("precise captures only through fields and derefs"),
-                });
+            // These projections are applied in order to "bridge" the local that we are
+            // currently transforming *from* the old upvar that the by-ref coroutine used
+            // to capture *to* the upvar of the parent coroutine-closure. For example, if
+            // the parent captures `&s` but the child captures `&(s.field)`, then we will
+            // apply a field projection.
+            let bridging_projections = bridging_projections.iter().map(|elem| match elem.kind {
+                ProjectionKind::Deref => mir::ProjectionElem::Deref,
+                ProjectionKind::Field(idx, VariantIdx::ZERO) => {
+                    mir::ProjectionElem::Field(idx, elem.ty)
+                }
+                _ => unreachable!("precise captures only through fields and derefs"),
+            });
 
             // We start out with an adjusted field index (and ty), representing the
             // upvar that we get from our parent closure. We apply any of the additional
@@ -326,8 +324,8 @@ impl<'tcx> MutVisitor<'tcx> for MakeByMoveBody<'tcx> {
                 projection: self.tcx.mk_place_elems_from_iter(
                     [mir::ProjectionElem::Field(remapped_idx, remapped_ty)]
                         .into_iter()
-                        .chain(additional_projections)
-                        .chain(final_deref.iter().copied()),
+                        .chain(bridging_projections)
+                        .chain(final_projections.iter().copied()),
                 ),
             };
         }
diff --git a/tests/ui/async-await/async-closures/truncated-fields-when-imm.rs b/tests/ui/async-await/async-closures/truncated-fields-when-imm.rs
new file mode 100644
index 00000000000..5c718638d80
--- /dev/null
+++ b/tests/ui/async-await/async-closures/truncated-fields-when-imm.rs
@@ -0,0 +1,17 @@
+//@ edition: 2021
+//@ check-pass
+
+#![feature(async_closure)]
+
+pub struct Struct {
+    pub path: String,
+}
+
+// In `upvar.rs`, `truncate_capture_for_optimization` means that we don't actually
+// capture `&(*s.path)` here, but instead just `&(*s)`, but ONLY when the upvar is
+// immutable. This means that the assumption we have in `ByMoveBody` pass is wrong.
+pub fn test(s: &Struct) {
+    let c = async move || { let path = &s.path; };
+}
+
+fn main() {}