Introduce new fake reads

compiler/rustc_middle/src/ty/context.rs

@@ -430,6 +430,9 @@ pub struct TypeckResults<'tcx> {
     /// see `MinCaptureInformationMap` for more details.
     pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>,
 
+    /// [FIXME] RFC2229 Change to use HashSet instead of Vec
+    pub closure_fake_reads: FxHashMap<DefId, Vec<HirPlace<'tcx>>>,
+
     /// Stores the type, expression, span and optional scope span of all types
     /// that are live across the yield of this generator (if a generator).
     pub generator_interior_types: ty::Binder<Vec<GeneratorInteriorTypeCause<'tcx>>>,
@@ -464,6 +467,7 @@ pub fn new(hir_owner: LocalDefId) -> TypeckResults<'tcx> {
             concrete_opaque_types: Default::default(),
             closure_captures: Default::default(),
             closure_min_captures: Default::default(),
+            closure_fake_reads: Default::default(),
             generator_interior_types: ty::Binder::dummy(Default::default()),
             treat_byte_string_as_slice: Default::default(),
         }
@@ -715,6 +719,7 @@ fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHas
             ref concrete_opaque_types,
             ref closure_captures,
             ref closure_min_captures,
+            ref closure_fake_reads,
             ref generator_interior_types,
             ref treat_byte_string_as_slice,
         } = *self;
@@ -750,6 +755,7 @@ fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHas
             concrete_opaque_types.hash_stable(hcx, hasher);
             closure_captures.hash_stable(hcx, hasher);
             closure_min_captures.hash_stable(hcx, hasher);
+            closure_fake_reads.hash_stable(hcx, hasher);
             generator_interior_types.hash_stable(hcx, hasher);
             treat_byte_string_as_slice.hash_stable(hcx, hasher);
         })

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

@@ -17,7 +17,7 @@
 use rustc_index::vec::Idx;
 
 /// The "outermost" place that holds this value.
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 crate enum PlaceBase {
     /// Denotes the start of a `Place`.
     Local(Local),
@@ -67,7 +67,7 @@
 ///
 /// This is used internally when building a place for an expression like `a.b.c`. The fields `b`
 /// and `c` can be progressively pushed onto the place builder that is created when converting `a`.
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 crate struct PlaceBuilder<'tcx> {
     base: PlaceBase,
     projection: Vec<PlaceElem<'tcx>>,
@@ -199,7 +199,7 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>(
     from_builder: PlaceBuilder<'tcx>,
     tcx: TyCtxt<'tcx>,
     typeck_results: &'a ty::TypeckResults<'tcx>,
-) -> Result<PlaceBuilder<'tcx>, HirId> {
+) -> Result<PlaceBuilder<'tcx>, PlaceBuilder<'tcx>> {
     match from_builder.base {
         PlaceBase::Local(_) => Ok(from_builder),
         PlaceBase::Upvar { var_hir_id, closure_def_id, closure_kind } => {
@@ -236,7 +236,7 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>(
                         var_hir_id, from_builder.projection,
                     );
                 }
-                return Err(var_hir_id);
+                return Err(upvar_resolved_place_builder);
             };
 
             let closure_ty = typeck_results
@@ -288,7 +288,7 @@ impl<'tcx> PlaceBuilder<'tcx> {
         if let PlaceBase::Local(local) = self.base {
             Place { local, projection: tcx.intern_place_elems(&self.projection) }
         } else {
-            self.expect_upvars_resolved(tcx, typeck_results).into_place(tcx, typeck_results)
+            self.try_upvars_resolved(tcx, typeck_results).into_place(tcx, typeck_results)
         }
     }
 
@@ -300,6 +300,17 @@ fn expect_upvars_resolved<'a>(
         to_upvars_resolved_place_builder(self, tcx, typeck_results).unwrap()
     }
 
+    fn try_upvars_resolved<'a>(
+        self,
+        tcx: TyCtxt<'tcx>,
+        typeck_results: &'a ty::TypeckResults<'tcx>,
+    ) -> PlaceBuilder<'tcx> {
+        match to_upvars_resolved_place_builder(self, tcx, typeck_results) {
+            Ok(upvars_resolved) => upvars_resolved,
+            Err(upvars_unresolved) => upvars_unresolved,
+        }
+    }
+
     crate fn base(&self) -> PlaceBase {
         self.base
     }

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

@@ -8,6 +8,7 @@
 use crate::thir::*;
 use rustc_middle::middle::region;
 use rustc_middle::mir::AssertKind;
+use rustc_middle::mir::Place;
 use rustc_middle::mir::*;
 use rustc_middle::ty::{self, Ty, UpvarSubsts};
 use rustc_span::Span;
@@ -164,7 +165,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                 block.and(Rvalue::Aggregate(box AggregateKind::Tuple, fields))
             }
-            ExprKind::Closure { closure_id, substs, upvars, movability } => {
+            ExprKind::Closure { closure_id, substs, upvars, movability, fake_reads } => {
                 // see (*) above
                 let operands: Vec<_> = upvars
                     .into_iter()
@@ -203,6 +204,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         }
                     })
                     .collect();
+
+                if let Some(fake_reads) = fake_reads {
+                    for thir_place in fake_reads.into_iter() {
+                        //  = this.hir.mirror(thir_place);
+                        let mir_place = unpack!(block = this.as_place(block, thir_place));
+                        // [FIXME] RFC2229 FakeReadCause can be ForLet or ForMatch, need to use the correct one
+                        this.cfg.push_fake_read(
+                            block,
+                            source_info,
+                            FakeReadCause::ForMatchedPlace,
+                            mir_place,
+                        );
+                    }
+                }
+
                 let result = match substs {
                     UpvarSubsts::Generator(substs) => {
                         // We implicitly set the discriminant to 0. See

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

@@ -420,7 +420,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             | ExprKind::PlaceTypeAscription { .. }
             | ExprKind::ValueTypeAscription { .. } => {
                 debug_assert!(Category::of(&expr.kind) == Some(Category::Place));
-
                 let place = unpack!(block = this.as_place(block, expr));
                 let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place));
                 this.cfg.push_assign(block, source_info, destination, rvalue);
@@ -437,7 +436,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 }
 
                 debug_assert!(Category::of(&expr.kind) == Some(Category::Place));
-
                 let place = unpack!(block = this.as_place(block, expr));
                 let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place));
                 this.cfg.push_assign(block, source_info, destination, rvalue);

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

@@ -5,6 +5,7 @@
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
 use rustc_index::vec::Idx;
+use rustc_middle::hir::place::Place as HirPlace;
 use rustc_middle::hir::place::PlaceBase as HirPlaceBase;
 use rustc_middle::hir::place::ProjectionKind as HirProjectionKind;
 use rustc_middle::mir::interpret::Scalar;
@@ -452,7 +453,39 @@ fn make_mirror_unadjusted(&mut self, expr: &'tcx hir::Expr<'tcx>) -> Expr<'thir,
                         .zip(substs.upvar_tys())
                         .map(|(captured_place, ty)| self.capture_upvar(expr, captured_place, ty)),
                 );
-                ExprKind::Closure { closure_id: def_id, substs, upvars, movability }
+
+                let fake_reads = match self.typeck_results().closure_fake_reads.get(&def_id) {
+                    Some(vals) => Some(self.arena.alloc_from_iter(vals
+                        .iter()
+                        .filter(|val| match val.base {
+                            HirPlaceBase::Upvar(_) => true,
+                            _ => false,
+                        })
+                        .map(|val| {
+                            let var_hir_id = match val.base {
+                                HirPlaceBase::Upvar(upvar_id) => {
+                                    debug!("upvar");
+                                    upvar_id.var_path.hir_id
+                                }
+                                _ => {
+                                    bug!(
+                                        "Do not know how to get HirId out of Rvalue and StaticItem"
+                                    );
+                                }
+                            };
+                            self.fake_read_capture_upvar(expr, val.clone(), var_hir_id)
+                        })
+                    )),
+                    None => None,
+                };
+
+                ExprKind::Closure {
+                    closure_id: def_id,
+                    substs,
+                    upvars,
+                    movability,
+                    fake_reads: fake_reads,
+                }
             }
 
             hir::ExprKind::Path(ref qpath) => {
@@ -1012,6 +1045,49 @@ fn overloaded_place(
         ExprKind::Deref { arg: ref_expr }
     }
 
+    fn fake_read_capture_upvar(
+        &mut self,
+        closure_expr: &'tcx hir::Expr<'tcx>,
+        place: HirPlace<'tcx>,
+        hir_id: hir::HirId,
+    ) -> Expr<'thir, 'tcx> {
+        let temp_lifetime = self.region_scope_tree.temporary_scope(closure_expr.hir_id.local_id);
+        let var_ty = place.base_ty;
+
+        let mut captured_place_expr = Expr {
+            temp_lifetime,
+            ty: var_ty,
+            span: closure_expr.span,
+            kind: self.convert_var(hir_id),
+        };
+        // [FIXME] RFC2229 Maybe we should introduce an immutable borrow of the fake capture so that we don't
+        // end up moving this place
+        for proj in place.projections.iter() {
+            let kind = match proj.kind {
+                HirProjectionKind::Deref => {
+                    ExprKind::Deref { arg: self.arena.alloc(captured_place_expr) }
+                }
+                HirProjectionKind::Field(field, ..) => {
+                    // Variant index will always be 0, because for multi-variant
+                    // enums, we capture the enum entirely.
+                    ExprKind::Field {
+                        lhs: self.arena.alloc(captured_place_expr),
+                        name: Field::new(field as usize),
+                    }
+                }
+                HirProjectionKind::Index | HirProjectionKind::Subslice => {
+                    // We don't capture these projections, so we can ignore them here
+                    continue;
+                }
+            };
+
+            captured_place_expr =
+                Expr { temp_lifetime, ty: proj.ty, span: closure_expr.span, kind };
+        }
+
+        captured_place_expr
+    }
+
     fn capture_upvar(
         &mut self,
         closure_expr: &'tcx hir::Expr<'tcx>,

compiler/rustc_mir_build/src/thir/mod.rs

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

compiler/rustc_typeck/src/check/upvar.rs

@@ -34,6 +34,7 @@
 use super::FnCtxt;
 
 use crate::expr_use_visitor as euv;
+use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
@@ -145,6 +146,7 @@ fn analyze_closure(
             current_closure_kind: ty::ClosureKind::LATTICE_BOTTOM,
             current_origin: None,
             capture_information: Default::default(),
+            fake_reads: Default::default(),
         };
         euv::ExprUseVisitor::new(
             &mut delegate,
@@ -246,6 +248,9 @@ fn analyze_closure(
         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(|fake_read| fake_read).collect();
+        self.typeck_results.borrow_mut().closure_fake_reads.insert(closure_def_id, fake_reads);
+
         // If we are also inferred the closure kind here,
         // process any deferred resolutions.
         let deferred_call_resolutions = self.remove_deferred_call_resolutions(closure_def_id);
@@ -1148,6 +1153,8 @@ struct InferBorrowKind<'a, 'tcx> {
     /// Place { V1, [ProjectionKind::Field(Index=1, Variant=0)] } : CaptureKind { E2, MutableBorrow }
     /// ```
     capture_information: InferredCaptureInformation<'tcx>,
+    // [FIXME] RFC2229 Change Vec to FxHashSet
+    fake_reads: FxHashSet<Place<'tcx>>, // these need to be fake read.
 }
 
 impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
@@ -1409,6 +1416,12 @@ fn init_capture_info_for_place(
 }
 
 impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
+    fn fake_read(&mut self, place: PlaceWithHirId<'tcx>) {
+        if let PlaceBase::Upvar(_) = place.place.base {
+            self.fake_reads.insert(place.place);
+        }
+    }
+
     fn consume(
         &mut self,
         place_with_id: &PlaceWithHirId<'tcx>,

compiler/rustc_typeck/src/check/writeback.rs

@@ -4,11 +4,14 @@
 
 use crate::check::FnCtxt;
 
+use rustc_data_structures::stable_map::FxHashMap;
 use rustc_errors::ErrorReported;
 use rustc_hir as hir;
+use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
 use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282;
 use rustc_infer::infer::InferCtxt;
+use rustc_middle::hir::place::Place as HirPlace;
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, PointerCast};
 use rustc_middle::ty::fold::{TypeFoldable, TypeFolder};
 use rustc_middle::ty::{self, Ty, TyCtxt};
@@ -56,6 +59,7 @@ pub fn resolve_type_vars_in_body(
         }
         wbcx.visit_body(body);
         wbcx.visit_min_capture_map();
+        wbcx.visit_fake_reads_map();
         wbcx.visit_upvar_capture_map();
         wbcx.visit_closures();
         wbcx.visit_liberated_fn_sigs();
@@ -363,6 +367,25 @@ fn visit_min_capture_map(&mut self) {
         self.typeck_results.closure_min_captures = min_captures_wb;
     }
 
+    fn visit_fake_reads_map(&mut self) {
+        let mut resolved_closure_fake_reads: FxHashMap<DefId, Vec<HirPlace<'tcx>>> =
+            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>>::new();
+            for fake_read 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(fake_read.clone(), &locatable);
+                resolved_fake_reads.push(resolved_fake_read);
+            }
+            resolved_closure_fake_reads.insert(*closure_def_id, resolved_fake_reads);
+        }
+        self.typeck_results.closure_fake_reads = resolved_closure_fake_reads;
+    }
+
     fn visit_upvar_capture_map(&mut self) {
         for (upvar_id, upvar_capture) in self.fcx.typeck_results.borrow().upvar_capture_map.iter() {
             let new_upvar_capture = match *upvar_capture {

compiler/rustc_typeck/src/expr_use_visitor.rs

@@ -10,6 +10,7 @@
 use rustc_hir as hir;
 use rustc_hir::def::Res;
 use rustc_hir::def_id::LocalDefId;
+// use rustc_hir::Pat;
 use rustc_hir::PatKind;
 use rustc_index::vec::Idx;
 use rustc_infer::infer::InferCtxt;
@@ -51,6 +52,9 @@ fn borrow(
     // The path at `assignee_place` is being assigned to.
     // `diag_expr_id` is the id used for diagnostics (see `consume` for more details).
     fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId);
+
+    // [FIXME] RFC2229 This should also affect clippy ref: https://github.com/sexxi-goose/rust/pull/27
+    fn fake_read(&mut self, place: PlaceWithHirId<'tcx>);
 }
 
 #[derive(Copy, Clone, PartialEq, Debug)]
@@ -229,7 +233,24 @@ pub fn walk_expr(&mut self, expr: &hir::Expr<'_>) {
 
             hir::ExprKind::Match(ref discr, arms, _) => {
                 let discr_place = return_if_err!(self.mc.cat_expr(&discr));
-                self.borrow_expr(&discr, ty::ImmBorrow);
+
+                // We only want to borrow discr if the pattern contain something other
+                // than wildcards
+                let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self;
+                let mut res = false;
+                for arm in arms.iter() {
+                    return_if_err!(mc.cat_pattern(discr_place.clone(), &arm.pat, |_place, pat| {
+                        if let PatKind::Binding(_, _, _, opt_sub_pat) = pat.kind {
+                            if let None = opt_sub_pat {
+                                res = true;
+                            }
+                        }
+                    }));
+                }
+
+                if res {
+                    self.borrow_expr(&discr, ty::ImmBorrow);
+                }
 
                 // treatment of the discriminant is handled while walking the arms.
                 for arm in arms {
@@ -537,6 +558,8 @@ fn walk_irrefutable_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir
     fn walk_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) {
         debug!("walk_pat(discr_place={:?}, pat={:?})", discr_place, pat);
 
+        self.delegate.fake_read(discr_place.clone());
+
         let tcx = self.tcx();
         let ExprUseVisitor { ref mc, body_owner: _, ref mut delegate } = *self;
         return_if_err!(mc.cat_pattern(discr_place.clone(), pat, |place, pat| {
@@ -599,6 +622,10 @@ fn walk_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) {
     fn walk_captures(&mut self, closure_expr: &hir::Expr<'_>) {
         debug!("walk_captures({:?})", closure_expr);
 
+        // Over here we walk a closure that is nested inside the current body
+        // If the current body is a closure, then we also want to report back any fake reads,
+        // starting off of variables that are captured by our parent as well.
+
         let closure_def_id = self.tcx().hir().local_def_id(closure_expr.hir_id).to_def_id();
         let upvars = self.tcx().upvars_mentioned(self.body_owner);
 
@@ -611,6 +638,7 @@ fn walk_captures(&mut self, closure_expr: &hir::Expr<'_>) {
         if let Some(min_captures) = self.mc.typeck_results.closure_min_captures.get(&closure_def_id)
         {
             for (var_hir_id, min_list) in min_captures.iter() {
+                // Use this as a reference for if we should promote the fake read
                 if upvars.map_or(body_owner_is_closure, |upvars| !upvars.contains_key(var_hir_id)) {
                     // The nested closure might be capturing the current (enclosing) closure's local variables.
                     // We check if the root variable is ever mentioned within the enclosing closure, if not
@@ -636,6 +664,12 @@ fn walk_captures(&mut self, closure_expr: &hir::Expr<'_>) {
                         place.projections.clone(),
                     );
 
+                    // [FIXME] RFC2229 We want to created another loop that iterates mc.typeck_results.fake_reads()
+                    // [FIXME] RFC2229 Add tests for nested closures
+                    if body_owner_is_closure {
+                        self.delegate.fake_read(place_with_id.clone());
+                    }
+
                     match capture_info.capture_kind {
                         ty::UpvarCapture::ByValue(_) => {
                             let mode = copy_or_move(&self.mc, &place_with_id);

src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.rs

@@ -13,12 +13,8 @@ fn main() {
     let mut point = SingleVariant::Point(10, -10);
 
     let c = || {
-        // FIXME(project-rfc-2229#24): Change this to be a destructure pattern
-        // once this is fixed, to remove the warning.
-        if let SingleVariant::Point(ref mut x, _) = point {
-            //~^ WARNING: irrefutable `if let` pattern
-            *x += 1;
-        }
+        let SingleVariant::Point(ref mut x, _) = point;
+        *x += 1;
     };
 
     let b = c;

src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr

@@ -21,7 +21,7 @@ LL | |         }
    = help: consider replacing the `if let` with a `let`
 
 error[E0382]: use of moved value: `c`
-  --> $DIR/closure-origin-single-variant-diagnostics.rs:25:13
+  --> $DIR/closure-origin-single-variant-diagnostics.rs:21:13
    |
 LL |     let b = c;
    |             - value moved here
@@ -29,11 +29,11 @@ LL |     let a = c;
    |             ^ value used here after move
    |
 note: closure cannot be moved more than once as it is not `Copy` due to moving the variable `point.0` out of its environment
-  --> $DIR/closure-origin-single-variant-diagnostics.rs:18:53
+  --> $DIR/closure-origin-single-variant-diagnostics.rs:16:50
    |
-LL |         if let SingleVariant::Point(ref mut x, _) = point {
-   |                                                     ^^^^^
+LL |         let SingleVariant::Point(ref mut x, _) = point;
+   |                                                  ^^^^^
 
-error: aborting due to previous error; 2 warnings emitted
+error: aborting due to previous error; 1 warning emitted
 
 For more information about this error, try `rustc --explain E0382`.