Rollup merge of rust-lang#80635 - sexxi-goose:use-place-instead-of-symbol, r=nikomatsakis`

Improve diagnostics when closure doesn't meet trait bound

Improves the diagnostics when closure doesn't meet trait bound by modifying `TypeckResuts::closure_kind_origins` such that `hir::Place` is used instead of `Symbol`. Using `hir::Place` to describe which capture influenced the decision of selecting a trait a closure satisfies to (Fn/FnMut/FnOnce, Copy) allows us to show precise path in the diagnostics when `capture_disjoint_field` feature is enabled.

Closes rust-lang/project-rfc-2229/issues/21

r? ```@nikomatsakis```

Author: m-ou-se

compiler/rustc_middle/src/ty/context.rs

@@ -3,6 +3,7 @@
 use crate::arena::Arena;
 use crate::dep_graph::DepGraph;
 use crate::hir::exports::ExportMap;
+use crate::hir::place::Place as HirPlace;
 use crate::ich::{NodeIdHashingMode, StableHashingContext};
 use crate::infer::canonical::{Canonical, CanonicalVarInfo, CanonicalVarInfos};
 use crate::lint::{struct_lint_level, LintDiagnosticBuilder, LintLevelSource};
@@ -379,7 +380,7 @@ pub struct TypeckResults<'tcx> {
 
     /// Records the reasons that we picked the kind of each closure;
     /// not all closures are present in the map.
-    closure_kind_origins: ItemLocalMap<(Span, Symbol)>,
+    closure_kind_origins: ItemLocalMap<(Span, HirPlace<'tcx>)>,
 
     /// For each fn, records the "liberated" types of its arguments
     /// and return type. Liberated means that all bound regions
@@ -642,11 +643,13 @@ impl<'tcx> TypeckResults<'tcx> {
         self.upvar_capture_map[&upvar_id]
     }
 
-    pub fn closure_kind_origins(&self) -> LocalTableInContext<'_, (Span, Symbol)> {
+    pub fn closure_kind_origins(&self) -> LocalTableInContext<'_, (Span, HirPlace<'tcx>)> {
         LocalTableInContext { hir_owner: self.hir_owner, data: &self.closure_kind_origins }
     }
 
-    pub fn closure_kind_origins_mut(&mut self) -> LocalTableInContextMut<'_, (Span, Symbol)> {
+    pub fn closure_kind_origins_mut(
+        &mut self,
+    ) -> LocalTableInContextMut<'_, (Span, HirPlace<'tcx>)> {
         LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.closure_kind_origins }
     }
 

compiler/rustc_middle/src/ty/mod.rs

@@ -17,7 +17,9 @@ pub use self::IntVarValue::*;
 pub use self::Variance::*;
 
 use crate::hir::exports::ExportMap;
-use crate::hir::place::Place as HirPlace;
+use crate::hir::place::{
+    Place as HirPlace, PlaceBase as HirPlaceBase, ProjectionKind as HirProjectionKind,
+};
 use crate::ich::StableHashingContext;
 use crate::middle::cstore::CrateStoreDyn;
 use crate::middle::resolve_lifetime::ObjectLifetimeDefault;
@@ -734,6 +736,43 @@ pub struct CapturedPlace<'tcx> {
     pub info: CaptureInfo<'tcx>,
 }
 
+pub fn place_to_string_for_capture(tcx: TyCtxt<'tcx>, place: &HirPlace<'tcx>) -> String {
+    let name = match place.base {
+        HirPlaceBase::Upvar(upvar_id) => tcx.hir().name(upvar_id.var_path.hir_id).to_string(),
+        _ => bug!("Capture_information should only contain upvars"),
+    };
+    let mut curr_string = name;
+
+    for (i, proj) in place.projections.iter().enumerate() {
+        match proj.kind {
+            HirProjectionKind::Deref => {
+                curr_string = format!("*{}", curr_string);
+            }
+            HirProjectionKind::Field(idx, variant) => match place.ty_before_projection(i).kind() {
+                ty::Adt(def, ..) => {
+                    curr_string = format!(
+                        "{}.{}",
+                        curr_string,
+                        def.variants[variant].fields[idx as usize].ident.name.as_str()
+                    );
+                }
+                ty::Tuple(_) => {
+                    curr_string = format!("{}.{}", curr_string, idx);
+                }
+                _ => {
+                    bug!(
+                        "Field projection applied to a type other than Adt or Tuple: {:?}.",
+                        place.ty_before_projection(i).kind()
+                    )
+                }
+            },
+            proj => bug!("{:?} unexpected because it isn't captured", proj),
+        }
+    }
+
+    curr_string.to_string()
+}
+
 /// Part of `MinCaptureInformationMap`; describes the capture kind (&, &mut, move)
 /// for a particular capture as well as identifying the part of the source code
 /// that triggered this capture to occur.

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

@@ -103,15 +103,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                         let did = did.expect_local();
                         let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(did);
 
-                        if let Some((span, name)) =
+                        if let Some((span, hir_place)) =
                             self.infcx.tcx.typeck(did).closure_kind_origins().get(hir_id)
                         {
                             diag.span_note(
                                 *span,
                                 &format!(
                                     "closure cannot be invoked more than once because it moves the \
                                     variable `{}` out of its environment",
-                                    name,
+                                    ty::place_to_string_for_capture(self.infcx.tcx, hir_place)
                                 ),
                             );
                             return;
@@ -127,15 +127,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 let did = did.expect_local();
                 let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(did);
 
-                if let Some((span, name)) =
+                if let Some((span, hir_place)) =
                     self.infcx.tcx.typeck(did).closure_kind_origins().get(hir_id)
                 {
                     diag.span_note(
                         *span,
                         &format!(
                             "closure cannot be moved more than once as it is not `Copy` due to \
                              moving the variable `{}` out of its environment",
-                            name
+                            ty::place_to_string_for_capture(self.infcx.tcx, hir_place)
                         ),
                     );
                 }

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -589,23 +589,23 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                         if let Some(typeck_results) = self.in_progress_typeck_results {
                             let typeck_results = typeck_results.borrow();
                             match (found_kind, typeck_results.closure_kind_origins().get(hir_id)) {
-                                (ty::ClosureKind::FnOnce, Some((span, name))) => {
+                                (ty::ClosureKind::FnOnce, Some((span, place))) => {
                                     err.span_label(
                                         *span,
                                         format!(
                                             "closure is `FnOnce` because it moves the \
                                          variable `{}` out of its environment",
-                                            name
+                                            ty::place_to_string_for_capture(tcx, place)
                                         ),
                                     );
                                 }
-                                (ty::ClosureKind::FnMut, Some((span, name))) => {
+                                (ty::ClosureKind::FnMut, Some((span, place))) => {
                                     err.span_label(
                                         *span,
                                         format!(
                                             "closure is `FnMut` because it mutates the \
                                          variable `{}` here",
-                                            name
+                                            ty::place_to_string_for_capture(tcx, place)
                                         ),
                                     );
                                 }

compiler/rustc_typeck/src/check/upvar.rs

@@ -176,7 +176,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             self.demand_eqtype(span, inferred_kind.to_ty(self.tcx), closure_kind_ty);
 
             // If we have an origin, store it.
-            if let Some(origin) = delegate.current_origin {
+            if let Some(origin) = delegate.current_origin.clone() {
+                let origin = if self.tcx.features().capture_disjoint_fields {
+                    origin
+                } else {
+                    // FIXME(project-rfc-2229#26): Once rust-lang#80092 is merged, we should restrict the
+                    // precision of origin as well. Otherwise, this will cause issues when project-rfc-2229#26
+                    // is fixed as we might see Index projections in the origin, which we can't print because
+                    // we don't store enough information.
+                    (origin.0, Place { projections: vec![], ..origin.1 })
+                };
+
                 self.typeck_results
                     .borrow_mut()
                     .closure_kind_origins_mut()
@@ -563,7 +573,7 @@ struct InferBorrowKind<'a, 'tcx> {
 
     // If we modified `current_closure_kind`, this field contains a `Some()` with the
     // variable access that caused us to do so.
-    current_origin: Option<(Span, Symbol)>,
+    current_origin: Option<(Span, Place<'tcx>)>,
 
     /// For each Place that is captured by the closure, we track the minimal kind of
     /// access we need (ref, ref mut, move, etc) and the expression that resulted in such access.
@@ -628,7 +638,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
             upvar_id.closure_expr_id,
             ty::ClosureKind::FnOnce,
             usage_span,
-            var_name(tcx, upvar_id.var_path.hir_id),
+            place_with_id.place.clone(),
         );
 
         let capture_info = ty::CaptureInfo {
@@ -720,7 +730,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
                 upvar_id.closure_expr_id,
                 ty::ClosureKind::FnMut,
                 tcx.hir().span(diag_expr_id),
-                var_name(tcx, upvar_id.var_path.hir_id),
+                place_with_id.place.clone(),
             );
         }
     }
@@ -765,11 +775,11 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
         closure_id: LocalDefId,
         new_kind: ty::ClosureKind,
         upvar_span: Span,
-        var_name: Symbol,
+        place: Place<'tcx>,
     ) {
         debug!(
-            "adjust_closure_kind(closure_id={:?}, new_kind={:?}, upvar_span={:?}, var_name={})",
-            closure_id, new_kind, upvar_span, var_name
+            "adjust_closure_kind(closure_id={:?}, new_kind={:?}, upvar_span={:?}, place={:?})",
+            closure_id, new_kind, upvar_span, place
         );
 
         // Is this the closure whose kind is currently being inferred?
@@ -797,7 +807,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
             | (ty::ClosureKind::FnMut, ty::ClosureKind::FnOnce) => {
                 // new kind is stronger than the old kind
                 self.current_closure_kind = new_kind;
-                self.current_origin = Some((upvar_span, var_name));
+                self.current_origin = Some((upvar_span, place));
             }
         }
     }

compiler/rustc_typeck/src/check/writeback.rs

@@ -384,9 +384,11 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
         assert_eq!(fcx_typeck_results.hir_owner, self.typeck_results.hir_owner);
         let common_hir_owner = fcx_typeck_results.hir_owner;
 
-        for (&id, &origin) in fcx_typeck_results.closure_kind_origins().iter() {
-            let hir_id = hir::HirId { owner: common_hir_owner, local_id: id };
-            self.typeck_results.closure_kind_origins_mut().insert(hir_id, origin);
+        for (id, origin) in fcx_typeck_results.closure_kind_origins().iter() {
+            let hir_id = hir::HirId { owner: common_hir_owner, local_id: *id };
+            let place_span = origin.0;
+            let place = self.resolve(origin.1.clone(), &place_span);
+            self.typeck_results.closure_kind_origins_mut().insert(hir_id, (place_span, place));
         }
     }
 

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

@@ -0,0 +1,31 @@
+#![feature(capture_disjoint_fields)]
+//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
+//~| `#[warn(incomplete_features)]` on by default
+//~| see issue #53488 <https://github.com/rust-lang/rust/issues/53488>
+
+// Check that precise paths are being reported back in the error message.
+
+
+enum MultiVariant {
+    Point(i32, i32),
+    Meta(i32)
+}
+
+fn main() {
+    let mut point = MultiVariant::Point(10, -10,);
+
+    let mut meta = MultiVariant::Meta(1);
+
+    let c = || {
+        if let MultiVariant::Point(ref mut x, _) = point {
+            *x += 1;
+        }
+
+        if let MultiVariant::Meta(ref mut v) = meta {
+            *v += 1;
+        }
+    };
+
+    let a = c;
+    let b = c; //~ ERROR use of moved value: `c` [E0382]
+}

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

@@ -0,0 +1,26 @@
+warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
+  --> $DIR/closure-origin-multi-variant-diagnostics.rs:1:12
+   |
+LL | #![feature(capture_disjoint_fields)]
+   |            ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(incomplete_features)]` on by default
+   = note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
+
+error[E0382]: use of moved value: `c`
+  --> $DIR/closure-origin-multi-variant-diagnostics.rs:30:13
+   |
+LL |     let a = c;
+   |             - value moved here
+LL |     let b = 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-multi-variant-diagnostics.rs:20:52
+   |
+LL |         if let MultiVariant::Point(ref mut x, _) = point {
+   |                                                    ^^^^^
+
+error: aborting due to previous error; 1 warning emitted
+
+For more information about this error, try `rustc --explain E0382`.

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

@@ -0,0 +1,26 @@
+#![feature(capture_disjoint_fields)]
+//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
+//~| `#[warn(incomplete_features)]` on by default
+//~| see issue #53488 <https://github.com/rust-lang/rust/issues/53488>
+
+// Check that precise paths are being reported back in the error message.
+
+enum SingleVariant {
+    Point(i32, i32),
+}
+
+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 b = c;
+    let a = c; //~ ERROR use of moved value: `c` [E0382]
+}

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

@@ -0,0 +1,37 @@
+warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
+  --> $DIR/closure-origin-single-variant-diagnostics.rs:1:12
+   |
+LL | #![feature(capture_disjoint_fields)]
+   |            ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(incomplete_features)]` on by default
+   = note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
+
+warning: irrefutable if-let pattern
+  --> $DIR/closure-origin-single-variant-diagnostics.rs:18:9
+   |
+LL | /         if let SingleVariant::Point(ref mut x, _) = point {
+LL | |
+LL | |             *x += 1;
+LL | |         }
+   | |_________^
+   |
+   = note: `#[warn(irrefutable_let_patterns)]` on by default
+
+error[E0382]: use of moved value: `c`
+  --> $DIR/closure-origin-single-variant-diagnostics.rs:25:13
+   |
+LL |     let b = c;
+   |             - value moved here
+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
+   |
+LL |         if let SingleVariant::Point(ref mut x, _) = point {
+   |                                                     ^^^^^
+
+error: aborting due to previous error; 2 warnings emitted
+
+For more information about this error, try `rustc --explain E0382`.