Fix diagnostic issue when using FakeReads in closures

Author: roxelo

compiler/rustc_middle/src/mir/mod.rs

@@ -1452,7 +1452,7 @@ pub struct Statement<'tcx> {
 
 // `Statement` is used a lot. Make sure it doesn't unintentionally get bigger.
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
-static_assert_size!(Statement<'_>, 32);
+static_assert_size!(Statement<'_>, 40);
 
 impl Statement<'_> {
     /// Changes a statement to a nop. This is both faster than deleting instructions and avoids
@@ -1575,7 +1575,12 @@ pub enum FakeReadCause {
 
     /// `let x: !; match x {}` doesn't generate any read of x so we need to
     /// generate a read of x to check that it is initialized and safe.
-    ForMatchedPlace,
+    ///
+    /// If a closure pattern matches a Place starting with an Upvar, then we introduce a
+    /// FakeRead for that Place outside the closure, in such a case this option would be
+    /// Some(closure_def_id).
+    /// Otherwise, the value of the optional DefId will be None.
+    ForMatchedPlace(Option<DefId>),
 
     /// A fake read of the RefWithinGuard version of a bind-by-value variable
     /// in a match guard to ensure that it's value hasn't change by the time
@@ -1594,7 +1599,12 @@ pub enum FakeReadCause {
     /// but in some cases it can affect the borrow checker, as in #53695.
     /// Therefore, we insert a "fake read" here to ensure that we get
     /// appropriate errors.
-    ForLet,
+    ///
+    /// If a closure pattern matches a Place starting with an Upvar, then we introduce a
+    /// FakeRead for that Place outside the closure, in such a case this option would be
+    /// Some(closure_def_id).
+    /// Otherwise, the value of the optional DefId will be None.
+    ForLet(Option<DefId>),
 
     /// If we have an index expression like
     ///

compiler/rustc_mir/src/borrow_check/diagnostics/explain_borrow.rs

@@ -515,7 +515,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 let block = &self.body.basic_blocks()[location.block];
 
                 let kind = if let Some(&Statement {
-                    kind: StatementKind::FakeRead(FakeReadCause::ForLet, _),
+                    kind: StatementKind::FakeRead(FakeReadCause::ForLet(_), _),
                     ..
                 }) = block.statements.get(location.statement_index)
                 {

compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs

@@ -7,8 +7,8 @@ use rustc_hir::def_id::DefId;
 use rustc_hir::lang_items::LangItemGroup;
 use rustc_hir::GeneratorKind;
 use rustc_middle::mir::{
-    AggregateKind, Constant, Field, Local, LocalInfo, LocalKind, Location, Operand, Place,
-    PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind,
+    AggregateKind, Constant, FakeReadCause, Field, Local, LocalInfo, LocalKind, Location, Operand,
+    Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind,
 };
 use rustc_middle::ty::print::Print;
 use rustc_middle::ty::{self, DefIdTree, Instance, Ty, TyCtxt};
@@ -795,6 +795,24 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             }
         }
 
+        // StatementKind::FakeRead only contains a def_id if they are introduced as a result
+        // of pattern matching within a closure.
+        if let StatementKind::FakeRead(cause, box ref place) = stmt.kind {
+            match cause {
+                FakeReadCause::ForMatchedPlace(Some(closure_def_id))
+                | FakeReadCause::ForLet(Some(closure_def_id)) => {
+                    debug!("move_spans: def_id={:?} place={:?}", closure_def_id, place);
+                    let places = &[Operand::Move(*place)];
+                    if let Some((args_span, generator_kind, var_span)) =
+                        self.closure_span(closure_def_id, moved_place, places)
+                    {
+                        return ClosureUse { generator_kind, args_span, var_span };
+                    }
+                }
+                _ => {}
+            }
+        }
+
         let normal_ret =
             if moved_place.projection.iter().any(|p| matches!(p, ProjectionElem::Downcast(..))) {
                 PatUse(stmt.source_info.span)

compiler/rustc_mir_build/src/build/expr/as_rvalue.rs

@@ -179,24 +179,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 //     match x { _ => () } // fake read of `x`
                 // };
                 // ```
-                // FIXME(RFC2229): Remove feature gate once diagnostics are improved
-                if this.tcx.features().capture_disjoint_fields {
-                    for (thir_place, cause, hir_id) in fake_reads.into_iter() {
-                        let place_builder =
-                            unpack!(block = this.as_place_builder(block, thir_place));
-
-                        if let Ok(place_builder_resolved) =
-                            place_builder.try_upvars_resolved(this.tcx, this.typeck_results)
-                        {
-                            let mir_place =
-                                place_builder_resolved.into_place(this.tcx, this.typeck_results);
-                            this.cfg.push_fake_read(
-                                block,
-                                this.source_info(this.tcx.hir().span(*hir_id)),
-                                *cause,
-                                mir_place,
-                            );
-                        }
+                for (thir_place, cause, hir_id) in fake_reads.into_iter() {
+                    let place_builder = unpack!(block = this.as_place_builder(block, thir_place));
+
+                    if let Ok(place_builder_resolved) =
+                        place_builder.try_upvars_resolved(this.tcx, this.typeck_results)
+                    {
+                        let mir_place =
+                            place_builder_resolved.into_place(this.tcx, this.typeck_results);
+                        this.cfg.push_fake_read(
+                            block,
+                            this.source_info(this.tcx.hir().span(*hir_id)),
+                            *cause,
+                            mir_place,
+                        );
                     }
                 }
 

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -139,7 +139,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         // uninhabited value. If we get never patterns, those will check that
         // the place is initialized, and so this read would only be used to
         // check safety.
-        let cause_matched_place = FakeReadCause::ForMatchedPlace;
+        let cause_matched_place = FakeReadCause::ForMatchedPlace(None);
         let source_info = self.source_info(scrutinee_span);
 
         if let Ok(scrutinee_builder) =
@@ -400,7 +400,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                 // Inject a fake read, see comments on `FakeReadCause::ForLet`.
                 let source_info = self.source_info(irrefutable_pat.span);
-                self.cfg.push_fake_read(block, source_info, FakeReadCause::ForLet, place);
+                self.cfg.push_fake_read(block, source_info, FakeReadCause::ForLet(None), place);
 
                 self.schedule_drop_for_binding(var, irrefutable_pat.span, OutsideGuard);
                 block.unit()
@@ -435,7 +435,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                 // Inject a fake read, see comments on `FakeReadCause::ForLet`.
                 let pattern_source_info = self.source_info(irrefutable_pat.span);
-                let cause_let = FakeReadCause::ForLet;
+                let cause_let = FakeReadCause::ForLet(None);
                 self.cfg.push_fake_read(block, pattern_source_info, cause_let, place);
 
                 let ty_source_info = self.source_info(user_ty_span);

compiler/rustc_typeck/src/expr_use_visitor.rs

@@ -280,9 +280,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
                 if needs_to_be_read {
                     self.borrow_expr(&discr, ty::ImmBorrow);
                 } else {
+                    let closure_def_id = match discr_place.place.base {
+                        PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
+                        _ => None,
+                    };
+
                     self.delegate.fake_read(
                         discr_place.place.clone(),
-                        FakeReadCause::ForMatchedPlace,
+                        FakeReadCause::ForMatchedPlace(closure_def_id),
                         discr_place.hir_id,
                     );
 
@@ -578,9 +583,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
     }
 
     fn walk_arm(&mut self, discr_place: &PlaceWithHirId<'tcx>, arm: &hir::Arm<'_>) {
+        let closure_def_id = match discr_place.place.base {
+            PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
+            _ => None,
+        };
+
         self.delegate.fake_read(
             discr_place.place.clone(),
-            FakeReadCause::ForMatchedPlace,
+            FakeReadCause::ForMatchedPlace(closure_def_id),
             discr_place.hir_id,
         );
         self.walk_pat(discr_place, &arm.pat);
@@ -595,9 +605,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
     /// Walks a pat that occurs in isolation (i.e., top-level of fn argument or
     /// let binding, and *not* a match arm or nested pat.)
     fn walk_irrefutable_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) {
+        let closure_def_id = match discr_place.place.base {
+            PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id.to_def_id()),
+            _ => None,
+        };
+
         self.delegate.fake_read(
             discr_place.place.clone(),
-            FakeReadCause::ForLet,
+            FakeReadCause::ForLet(closure_def_id),
             discr_place.hir_id,
         );
         self.walk_pat(discr_place, pat);

src/test/mir-opt/address_of.address_of_reborrow.SimplifyCfg-initial.after.mir

@@ -130,12 +130,12 @@ fn address_of_reborrow() -> () {
         StorageLive(_2);                 // scope 0 at $DIR/address-of.rs:4:14: 4:21
         _2 = [const 0_i32; 10];          // scope 0 at $DIR/address-of.rs:4:14: 4:21
         _1 = &_2;                        // scope 0 at $DIR/address-of.rs:4:13: 4:21
-        FakeRead(ForLet, _1);            // scope 0 at $DIR/address-of.rs:4:9: 4:10
+        FakeRead(ForLet(None), _1);      // scope 0 at $DIR/address-of.rs:4:9: 4:10
         StorageLive(_3);                 // scope 1 at $DIR/address-of.rs:5:9: 5:14
         StorageLive(_4);                 // scope 1 at $DIR/address-of.rs:5:22: 5:29
         _4 = [const 0_i32; 10];          // scope 1 at $DIR/address-of.rs:5:22: 5:29
         _3 = &mut _4;                    // scope 1 at $DIR/address-of.rs:5:17: 5:29
-        FakeRead(ForLet, _3);            // scope 1 at $DIR/address-of.rs:5:9: 5:14
+        FakeRead(ForLet(None), _3);      // scope 1 at $DIR/address-of.rs:5:9: 5:14
         StorageLive(_5);                 // scope 2 at $DIR/address-of.rs:7:5: 7:18
         StorageLive(_6);                 // scope 2 at $DIR/address-of.rs:7:5: 7:18
         _6 = &raw const (*_1);           // scope 2 at $DIR/address-of.rs:7:5: 7:6
@@ -170,25 +170,25 @@ fn address_of_reborrow() -> () {
         StorageDead(_13);                // scope 2 at $DIR/address-of.rs:11:20: 11:21
         StorageLive(_15);                // scope 2 at $DIR/address-of.rs:13:9: 13:10
         _15 = &raw const (*_1);          // scope 2 at $DIR/address-of.rs:13:23: 13:24
-        FakeRead(ForLet, _15);           // scope 2 at $DIR/address-of.rs:13:9: 13:10
+        FakeRead(ForLet(None), _15);     // scope 2 at $DIR/address-of.rs:13:9: 13:10
         AscribeUserType(_15, o, UserTypeProjection { base: UserType(3), projs: [] }); // scope 2 at $DIR/address-of.rs:13:12: 13:20
         StorageLive(_16);                // scope 3 at $DIR/address-of.rs:14:9: 14:10
         _16 = &raw const (*_1);          // scope 3 at $DIR/address-of.rs:14:31: 14:32
-        FakeRead(ForLet, _16);           // scope 3 at $DIR/address-of.rs:14:9: 14:10
+        FakeRead(ForLet(None), _16);     // scope 3 at $DIR/address-of.rs:14:9: 14:10
         AscribeUserType(_16, o, UserTypeProjection { base: UserType(5), projs: [] }); // scope 3 at $DIR/address-of.rs:14:12: 14:28
         StorageLive(_17);                // scope 4 at $DIR/address-of.rs:15:9: 15:10
         StorageLive(_18);                // scope 4 at $DIR/address-of.rs:15:30: 15:31
         _18 = &raw const (*_1);          // scope 4 at $DIR/address-of.rs:15:30: 15:31
         _17 = move _18 as *const dyn std::marker::Send (Pointer(Unsize)); // scope 4 at $DIR/address-of.rs:15:30: 15:31
         StorageDead(_18);                // scope 4 at $DIR/address-of.rs:15:30: 15:31
-        FakeRead(ForLet, _17);           // scope 4 at $DIR/address-of.rs:15:9: 15:10
+        FakeRead(ForLet(None), _17);     // scope 4 at $DIR/address-of.rs:15:9: 15:10
         AscribeUserType(_17, o, UserTypeProjection { base: UserType(7), projs: [] }); // scope 4 at $DIR/address-of.rs:15:12: 15:27
         StorageLive(_19);                // scope 5 at $DIR/address-of.rs:16:9: 16:10
         StorageLive(_20);                // scope 5 at $DIR/address-of.rs:16:27: 16:28
         _20 = &raw const (*_1);          // scope 5 at $DIR/address-of.rs:16:27: 16:28
         _19 = move _20 as *const [i32] (Pointer(Unsize)); // scope 5 at $DIR/address-of.rs:16:27: 16:28
         StorageDead(_20);                // scope 5 at $DIR/address-of.rs:16:27: 16:28
-        FakeRead(ForLet, _19);           // scope 5 at $DIR/address-of.rs:16:9: 16:10
+        FakeRead(ForLet(None), _19);     // scope 5 at $DIR/address-of.rs:16:9: 16:10
         AscribeUserType(_19, o, UserTypeProjection { base: UserType(9), projs: [] }); // scope 5 at $DIR/address-of.rs:16:12: 16:24
         StorageLive(_21);                // scope 6 at $DIR/address-of.rs:18:5: 18:18
         StorageLive(_22);                // scope 6 at $DIR/address-of.rs:18:5: 18:18
@@ -218,25 +218,25 @@ fn address_of_reborrow() -> () {
         StorageDead(_27);                // scope 6 at $DIR/address-of.rs:21:22: 21:23
         StorageLive(_29);                // scope 6 at $DIR/address-of.rs:23:9: 23:10
         _29 = &raw const (*_3);          // scope 6 at $DIR/address-of.rs:23:23: 23:24
-        FakeRead(ForLet, _29);           // scope 6 at $DIR/address-of.rs:23:9: 23:10
+        FakeRead(ForLet(None), _29);     // scope 6 at $DIR/address-of.rs:23:9: 23:10
         AscribeUserType(_29, o, UserTypeProjection { base: UserType(13), projs: [] }); // scope 6 at $DIR/address-of.rs:23:12: 23:20
         StorageLive(_30);                // scope 7 at $DIR/address-of.rs:24:9: 24:10
         _30 = &raw const (*_3);          // scope 7 at $DIR/address-of.rs:24:31: 24:32
-        FakeRead(ForLet, _30);           // scope 7 at $DIR/address-of.rs:24:9: 24:10
+        FakeRead(ForLet(None), _30);     // scope 7 at $DIR/address-of.rs:24:9: 24:10
         AscribeUserType(_30, o, UserTypeProjection { base: UserType(15), projs: [] }); // scope 7 at $DIR/address-of.rs:24:12: 24:28
         StorageLive(_31);                // scope 8 at $DIR/address-of.rs:25:9: 25:10
         StorageLive(_32);                // scope 8 at $DIR/address-of.rs:25:30: 25:31
         _32 = &raw const (*_3);          // scope 8 at $DIR/address-of.rs:25:30: 25:31
         _31 = move _32 as *const dyn std::marker::Send (Pointer(Unsize)); // scope 8 at $DIR/address-of.rs:25:30: 25:31
         StorageDead(_32);                // scope 8 at $DIR/address-of.rs:25:30: 25:31
-        FakeRead(ForLet, _31);           // scope 8 at $DIR/address-of.rs:25:9: 25:10
+        FakeRead(ForLet(None), _31);     // scope 8 at $DIR/address-of.rs:25:9: 25:10
         AscribeUserType(_31, o, UserTypeProjection { base: UserType(17), projs: [] }); // scope 8 at $DIR/address-of.rs:25:12: 25:27
         StorageLive(_33);                // scope 9 at $DIR/address-of.rs:26:9: 26:10
         StorageLive(_34);                // scope 9 at $DIR/address-of.rs:26:27: 26:28
         _34 = &raw const (*_3);          // scope 9 at $DIR/address-of.rs:26:27: 26:28
         _33 = move _34 as *const [i32] (Pointer(Unsize)); // scope 9 at $DIR/address-of.rs:26:27: 26:28
         StorageDead(_34);                // scope 9 at $DIR/address-of.rs:26:27: 26:28
-        FakeRead(ForLet, _33);           // scope 9 at $DIR/address-of.rs:26:9: 26:10
+        FakeRead(ForLet(None), _33);     // scope 9 at $DIR/address-of.rs:26:9: 26:10
         AscribeUserType(_33, o, UserTypeProjection { base: UserType(19), projs: [] }); // scope 9 at $DIR/address-of.rs:26:12: 26:24
         StorageLive(_35);                // scope 10 at $DIR/address-of.rs:28:5: 28:16
         StorageLive(_36);                // scope 10 at $DIR/address-of.rs:28:5: 28:16
@@ -266,25 +266,25 @@ fn address_of_reborrow() -> () {
         StorageDead(_41);                // scope 10 at $DIR/address-of.rs:31:20: 31:21
         StorageLive(_43);                // scope 10 at $DIR/address-of.rs:33:9: 33:10
         _43 = &raw mut (*_3);            // scope 10 at $DIR/address-of.rs:33:21: 33:22
-        FakeRead(ForLet, _43);           // scope 10 at $DIR/address-of.rs:33:9: 33:10
+        FakeRead(ForLet(None), _43);     // scope 10 at $DIR/address-of.rs:33:9: 33:10
         AscribeUserType(_43, o, UserTypeProjection { base: UserType(23), projs: [] }); // scope 10 at $DIR/address-of.rs:33:12: 33:18
         StorageLive(_44);                // scope 11 at $DIR/address-of.rs:34:9: 34:10
         _44 = &raw mut (*_3);            // scope 11 at $DIR/address-of.rs:34:29: 34:30
-        FakeRead(ForLet, _44);           // scope 11 at $DIR/address-of.rs:34:9: 34:10
+        FakeRead(ForLet(None), _44);     // scope 11 at $DIR/address-of.rs:34:9: 34:10
         AscribeUserType(_44, o, UserTypeProjection { base: UserType(25), projs: [] }); // scope 11 at $DIR/address-of.rs:34:12: 34:26
         StorageLive(_45);                // scope 12 at $DIR/address-of.rs:35:9: 35:10
         StorageLive(_46);                // scope 12 at $DIR/address-of.rs:35:28: 35:29
         _46 = &raw mut (*_3);            // scope 12 at $DIR/address-of.rs:35:28: 35:29
         _45 = move _46 as *mut dyn std::marker::Send (Pointer(Unsize)); // scope 12 at $DIR/address-of.rs:35:28: 35:29
         StorageDead(_46);                // scope 12 at $DIR/address-of.rs:35:28: 35:29
-        FakeRead(ForLet, _45);           // scope 12 at $DIR/address-of.rs:35:9: 35:10
+        FakeRead(ForLet(None), _45);     // scope 12 at $DIR/address-of.rs:35:9: 35:10
         AscribeUserType(_45, o, UserTypeProjection { base: UserType(27), projs: [] }); // scope 12 at $DIR/address-of.rs:35:12: 35:25
         StorageLive(_47);                // scope 13 at $DIR/address-of.rs:36:9: 36:10
         StorageLive(_48);                // scope 13 at $DIR/address-of.rs:36:25: 36:26
         _48 = &raw mut (*_3);            // scope 13 at $DIR/address-of.rs:36:25: 36:26
         _47 = move _48 as *mut [i32] (Pointer(Unsize)); // scope 13 at $DIR/address-of.rs:36:25: 36:26
         StorageDead(_48);                // scope 13 at $DIR/address-of.rs:36:25: 36:26
-        FakeRead(ForLet, _47);           // scope 13 at $DIR/address-of.rs:36:9: 36:10
+        FakeRead(ForLet(None), _47);     // scope 13 at $DIR/address-of.rs:36:9: 36:10
         AscribeUserType(_47, o, UserTypeProjection { base: UserType(29), projs: [] }); // scope 13 at $DIR/address-of.rs:36:12: 36:22
         _0 = const ();                   // scope 0 at $DIR/address-of.rs:3:26: 37:2
         StorageDead(_47);                // scope 13 at $DIR/address-of.rs:37:1: 37:2