Add comments with examples and tests

Author: roxelo

compiler/rustc_middle/src/ty/context.rs

@@ -431,7 +431,29 @@ pub struct TypeckResults<'tcx> {
     /// see `MinCaptureInformationMap` for more details.
     pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>,
 
-    pub closure_fake_reads: FxHashMap<DefId, Vec<(HirPlace<'tcx>, FakeReadCause)>>,
+    /// Tracks the fake reads required for a closure and the reason for the fake read.
+    /// When performing pattern matching for closures, there are times we don't end up
+    /// reading places that are mentioned in a closure (because of _ patterns). However,
+    /// to ensure the places are initialized, we introduce fake reads.
+    /// Consider these two examples:
+    /// ``` (discriminant matching with only wildcard arm)
+    /// let x: u8;
+    /// let c = || match x { _ => () };
+    /// ```
+    /// In this example, we don't need to actually read/borrow `x` in `c`, and so we don't
+    /// want to capture it. However, we do still want an error here, because `x` should have
+    /// to be initialized at the point where c is created. Therefore, we add a "fake read"
+    /// instead.
+    /// ``` (destructured assignments)
+    /// let c = || {
+    ///     let (t1, t2) = t;
+    /// }
+    /// ```
+    /// In the second example, we capture the disjoint fields of `t` (`t.0` & `t.1`), but
+    /// we never capture `t`. This becomes an issue when we build MIR as we require
+    /// information on `t` in order to create place `t.0` and `t.1`. We can solve this
+    /// issue by fake reading `t`.
+    pub closure_fake_reads: FxHashMap<DefId, Vec<(HirPlace<'tcx>, FakeReadCause, hir::HirId)>>,
 
     /// Stores the type, expression, span and optional scope span of all types
     /// that are live across the yield of this generator (if a generator).

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

@@ -90,15 +90,13 @@ fn convert_to_hir_projections_and_truncate_for_capture<'tcx>(
         let hir_projection = match mir_projection {
             ProjectionElem::Deref => HirProjectionKind::Deref,
             ProjectionElem::Field(field, _) => {
-                // We will never encouter this for multivariant enums,
-                // read the comment for `Downcast`.
                 let variant = variant.unwrap_or(VariantIdx::new(0));
                 HirProjectionKind::Field(field.index() as u32, variant)
             }
             ProjectionElem::Downcast(.., idx) => {
-                // This projections exist for enums that have
-                // single and multiple variants.
-                // For single variants, enums are not captured completely.
+                // We don't expect to see multi-variant enums here, as earlier
+                // phases will have truncated them already. However, there can
+                // still be downcasts, thanks to single-variant enums.
                 // We keep track of VariantIdx so we can use this information
                 // if the next ProjectionElem is a Field.
                 variant = Some(*idx);
@@ -200,7 +198,7 @@ fn find_capture_matching_projections<'a, 'tcx>(
 /// Takes a PlaceBuilder and resolves the upvar (if any) within it, so that the
 /// `PlaceBuilder` now starts from `PlaceBase::Local`.
 ///
-/// Returns a Result with the error being the HirId of the Upvar that was not found.
+/// Returns a Result with the error being the PlaceBuilder (`from_builder`) that was not found.
 fn to_upvars_resolved_place_builder<'a, 'tcx>(
     from_builder: PlaceBuilder<'tcx>,
     tcx: TyCtxt<'tcx>,
@@ -305,15 +303,23 @@ impl<'tcx> PlaceBuilder<'tcx> {
         to_upvars_resolved_place_builder(self, tcx, typeck_results).unwrap()
     }
 
+    /// Attempts to resolve the `PlaceBuilder`.
+    /// On success, it will return the resolved `PlaceBuilder`.
+    /// On failure, it will return itself.
+    ///
+    /// Upvars resolve may fail for a `PlaceBuilder` when attempting to
+    /// resolve a disjoint field whose root variable is not captured
+    /// (destructured assignments) or when attempting to resolve a root
+    /// variable (discriminant matching with only wildcard arm) that is
+    /// not captured. This can happen because the final mir that will be
+    /// generated doesn't require a read for this place. Failures will only
+    /// happen inside closures.
     crate fn try_upvars_resolved<'a>(
         self,
         tcx: TyCtxt<'tcx>,
         typeck_results: &'a ty::TypeckResults<'tcx>,
     ) -> Result<PlaceBuilder<'tcx>, PlaceBuilder<'tcx>> {
-        match to_upvars_resolved_place_builder(self, tcx, typeck_results) {
-            Ok(upvars_resolved) => Ok(upvars_resolved),
-            Err(upvars_unresolved) => Err(upvars_unresolved),
-        }
+        to_upvars_resolved_place_builder(self, tcx, typeck_results)
     }
 
     crate fn base(&self) -> PlaceBase {
@@ -662,7 +668,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             block,
             source_info,
             len,
-            Rvalue::Len(slice.clone().into_place(self.tcx, self.typeck_results)),
+            Rvalue::Len(slice.into_place(self.tcx, self.typeck_results)),
         );
         // lt = idx < len
         self.cfg.push_assign(

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

@@ -165,13 +165,42 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                 block.and(Rvalue::Aggregate(box AggregateKind::Tuple, fields))
             }
-            ExprKind::Closure {
-                closure_id,
-                substs,
-                upvars,
-                movability,
-                fake_reads: opt_fake_reads,
-            } => {
+            ExprKind::Closure { closure_id, substs, upvars, movability, fake_reads } => {
+                // Convert the closure fake reads, if any, from `ExprRef` to mir `Place`
+                // and push the fake reads.
+                // This must come before creating the operands. This is required in case
+                // there is a fake read and a borrow of the same path, since otherwise the
+                // fake read might interfere with the borrow. Consider an example like this
+                // one:
+                // ```
+                // let mut x = 0;
+                // let c = || {
+                //     &mut x; // mutable borrow of `x`
+                //     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,
+                            );
+                        }
+                    }
+                }
+
                 // see (*) above
                 let operands: Vec<_> = upvars
                     .into_iter()
@@ -210,21 +239,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         }
                     })
                     .collect();
-                if let Some(fake_reads) = opt_fake_reads {
-                    for (thir_place, cause) 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.clone().try_upvars_resolved(this.tcx, this.typeck_results)
-                        {
-                            let mir_place = place_builder_resolved
-                                .clone()
-                                .into_place(this.tcx, this.typeck_results);
-                            this.cfg.push_fake_read(block, source_info, cause, mir_place);
-                        }
-                    }
-                }
 
                 let result = match substs {
                     UpvarSubsts::Generator(substs) => {

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

@@ -146,8 +146,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         if let Ok(scrutinee_builder) =
             scrutinee_place_builder.clone().try_upvars_resolved(self.tcx, self.typeck_results)
         {
-            let scrutinee_place =
-                scrutinee_builder.clone().into_place(self.tcx, self.typeck_results);
+            let scrutinee_place = scrutinee_builder.into_place(self.tcx, self.typeck_results);
             self.cfg.push_fake_read(block, source_info, cause_matched_place, scrutinee_place);
         }
 
@@ -245,7 +244,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let arm_source_info = self.source_info(arm.span);
                 let arm_scope = (arm.scope, arm_source_info);
                 self.in_scope(arm_scope, arm.lint_level, |this| {
-                    let body = arm.body.clone();
+                    let body = arm.body;
+
+                    // `try_upvars_resolved` may fail if it is unable to resolve the given
+                    // `PlaceBuilder` inside a closure. In this case, we don't want to include
+                    // a scrutinee place. `scrutinee_place_builder` will fail to be resolved
+                    // if the only match arm is a wildcard (`_`).
+                    // Example:
+                    // ```
+                    // let foo = (0, 1);
+                    // let c = || {
+                    //    match foo { _ => () };
+                    // };
+                    // ```
                     let mut opt_scrutinee_place: Option<(Option<&Place<'tcx>>, Span)> = None;
                     let scrutinee_place: Place<'tcx>;
                     if let Ok(scrutinee_builder) = scrutinee_place_builder
@@ -496,6 +507,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         VarBindingForm { opt_match_place: Some((ref mut match_place, _)), .. },
                     )))) = self.local_decls[local].local_info
                     {
+                        // `try_upvars_resolved` may fail if it is unable to resolve the given
+                        // `PlaceBuilder` inside a closure. In this case, we don't want to include
+                        // a scrutinee place. `scrutinee_place_builder` will fail for destructured
+                        // assignments. This is because a closure only captures the precise places
+                        // that it will read and as a result a closure may not capture the entire
+                        // tuple/struct and rather have individual places that will be read in the
+                        // final MIR.
+                        // Example:
+                        // ```
+                        // let foo = (0, 1);
+                        // let c = || {
+                        //    let (v1, v2) = foo;
+                        // };
+                        // ```
                         if let Ok(match_pair_resolved) =
                             initializer.clone().try_upvars_resolved(self.tcx, self.typeck_results)
                         {

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

@@ -160,7 +160,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         if let Ok(test_place_builder) =
             place_builder.clone().try_upvars_resolved(self.tcx, self.typeck_results)
         {
-            place = test_place_builder.clone().into_place(self.tcx, self.typeck_results);
+            place = test_place_builder.into_place(self.tcx, self.typeck_results);
         } else {
             return;
         }

compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -454,20 +454,20 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> {
                         .map(|(captured_place, ty)| self.capture_upvar(expr, captured_place, ty)),
                 );
 
+                // Convert the closure fake reads, if any, from hir `Place` to ExprRef
                 let fake_reads = match self.typeck_results().closure_fake_reads.get(&def_id) {
-                    Some(vals) => Some(
-                        vals.iter()
-                            .map(|(place, cause)| {
-                                (
-                                    self.arena.alloc(
-                                        self.convert_captured_hir_place(expr, place.clone()),
-                                    ),
-                                    *cause,
-                                )
-                            })
-                            .collect(),
-                    ),
-                    None => None,
+                    Some(fake_reads) => fake_reads
+                        .iter()
+                        .map(|(place, cause, hir_id)| {
+                            (
+                                self.arena
+                                    .alloc(self.convert_captured_hir_place(expr, place.clone())),
+                                *cause,
+                                *hir_id,
+                            )
+                        })
+                        .collect(),
+                    None => Vec::new(),
                 };
 
                 ExprKind::Closure {

compiler/rustc_mir_build/src/thir/mod.rs

@@ -281,7 +281,7 @@ pub enum ExprKind<'thir, 'tcx> {
         substs: UpvarSubsts<'tcx>,
         upvars: &'thir [Expr<'thir, 'tcx>],
         movability: Option<hir::Movability>,
-        fake_reads: Option<Vec<(&'thir mut Expr<'thir, 'tcx>, FakeReadCause)>>,
+        fake_reads: Vec<(&'thir mut Expr<'thir, 'tcx>, FakeReadCause, hir::HirId)>,
     },
     Literal {
         literal: &'tcx Const<'tcx>,

compiler/rustc_typeck/src/check/upvar.rs

@@ -248,8 +248,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let final_tupled_upvars_type = self.tcx.mk_tup(final_upvar_tys.iter());
         self.demand_suptype(span, substs.tupled_upvars_ty(), final_tupled_upvars_type);
 
-        let fake_reads =
-            delegate.fake_reads.into_iter().map(|(place, cause)| (place, cause)).collect();
+        let fake_reads = delegate
+            .fake_reads
+            .into_iter()
+            .map(|(place, cause, hir_id)| (place, cause, hir_id))
+            .collect();
         self.typeck_results.borrow_mut().closure_fake_reads.insert(closure_def_id, fake_reads);
 
         // If we are also inferred the closure kind here,
@@ -1154,7 +1157,7 @@ struct InferBorrowKind<'a, 'tcx> {
     /// Place { V1, [ProjectionKind::Field(Index=1, Variant=0)] } : CaptureKind { E2, MutableBorrow }
     /// ```
     capture_information: InferredCaptureInformation<'tcx>,
-    fake_reads: Vec<(Place<'tcx>, FakeReadCause)>,
+    fake_reads: Vec<(Place<'tcx>, FakeReadCause, hir::HirId)>,
 }
 
 impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
@@ -1416,9 +1419,9 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
-    fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause) {
+    fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id: hir::HirId) {
         if let PlaceBase::Upvar(_) = place.base {
-            self.fake_reads.push((place, cause));
+            self.fake_reads.push((place, cause, diag_expr_id));
         }
     }
 

compiler/rustc_typeck/src/check/writeback.rs

@@ -371,18 +371,18 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
     fn visit_fake_reads_map(&mut self) {
         let mut resolved_closure_fake_reads: FxHashMap<
             DefId,
-            Vec<(HirPlace<'tcx>, FakeReadCause)>,
+            Vec<(HirPlace<'tcx>, FakeReadCause, hir::HirId)>,
         > = Default::default();
         for (closure_def_id, fake_reads) in
             self.fcx.typeck_results.borrow().closure_fake_reads.iter()
         {
-            let mut resolved_fake_reads = Vec::<(HirPlace<'tcx>, FakeReadCause)>::new();
-            for (place, cause) in fake_reads.iter() {
+            let mut resolved_fake_reads = Vec::<(HirPlace<'tcx>, FakeReadCause, hir::HirId)>::new();
+            for (place, cause, hir_id) in fake_reads.iter() {
                 let locatable =
                     self.tcx().hir().local_def_id_to_hir_id(closure_def_id.expect_local());
 
                 let resolved_fake_read = self.resolve(place.clone(), &locatable);
-                resolved_fake_reads.push((resolved_fake_read, *cause));
+                resolved_fake_reads.push((resolved_fake_read, *cause, *hir_id));
             }
             resolved_closure_fake_reads.insert(*closure_def_id, resolved_fake_reads);
         }