Rollup merge of rust-lang#84358 - sexxi-goose:print_captures_borrowck_rebased, r=nikomatsakis

Update closure capture error logging for disjoint captures for disjoint captures

Improved error logging when `#![feature(capture_disjoint_fields)]` is used.

Closes rust-lang/project-rfc-2229#8
Closes rust-lang/project-rfc-2229#36
Closes rust-lang/project-rfc-2229#39
Closes rust-lang#76005

Author: Dylan-DPC

compiler/rustc_middle/src/mir/mod.rs

@@ -683,6 +683,15 @@ impl BorrowKind {
             BorrowKind::Mut { allow_two_phase_borrow } => allow_two_phase_borrow,
         }
     }
+
+    pub fn describe_mutability(&self) -> String {
+        match *self {
+            BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => {
+                "immutable".to_string()
+            }
+            BorrowKind::Mut { .. } => "mutable".to_string(),
+        }
+    }
 }
 
 ///////////////////////////////////////////////////////////////////////////
@@ -2369,6 +2378,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
                             };
                             let mut struct_fmt = fmt.debug_struct(&name);
 
+                            // FIXME(project-rfc-2229#48): This should be a list of capture names/places
                             if let Some(upvars) = tcx.upvars_mentioned(def_id) {
                                 for (&var_id, place) in iter::zip(upvars.keys(), places) {
                                     let var_name = tcx.hir().name(var_id);
@@ -2388,6 +2398,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
                             let name = format!("[generator@{:?}]", tcx.hir().span(hir_id));
                             let mut struct_fmt = fmt.debug_struct(&name);
 
+                            // FIXME(project-rfc-2229#48): This should be a list of capture names/places
                             if let Some(upvars) = tcx.upvars_mentioned(def_id) {
                                 for (&var_id, place) in iter::zip(upvars.keys(), places) {
                                     let var_name = tcx.hir().name(var_id);

compiler/rustc_middle/src/ty/closure.rs

@@ -151,6 +151,10 @@ pub struct CapturedPlace<'tcx> {
 }
 
 impl CapturedPlace<'tcx> {
+    pub fn to_string(&self, tcx: TyCtxt<'tcx>) -> String {
+        place_to_string_for_capture(tcx, &self.place)
+    }
+
     /// Returns the hir-id of the root variable for the captured place.
     /// e.g., if `a.b.c` was captured, would return the hir-id for `a`.
     pub fn get_root_variable(&self) -> hir::HirId {
@@ -168,6 +172,22 @@ impl CapturedPlace<'tcx> {
         }
     }
 
+    /// Return span pointing to use that resulted in selecting the captured path
+    pub fn get_path_span(&self, tcx: TyCtxt<'tcx>) -> Span {
+        if let Some(path_expr_id) = self.info.path_expr_id {
+            tcx.hir().span(path_expr_id)
+        } else if let Some(capture_kind_expr_id) = self.info.capture_kind_expr_id {
+            tcx.hir().span(capture_kind_expr_id)
+        } else {
+            // Fallback on upvars mentioned if neither path or capture expr id is captured
+
+            // Safe to unwrap since we know this place is captured by the closure, therefore the closure must have upvars.
+            tcx.upvars_mentioned(self.get_closure_local_def_id()).unwrap()
+                [&self.get_root_variable()]
+                .span
+        }
+    }
+
     /// Return span pointing to use that resulted in selecting the current capture kind
     pub fn get_capture_kind_span(&self, tcx: TyCtxt<'tcx>) -> Span {
         if let Some(capture_kind_expr_id) = self.info.capture_kind_expr_id {

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

@@ -99,7 +99,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             );
             err.span_label(span, format!("use of possibly-uninitialized {}", item_msg));
 
-            use_spans.var_span_label(
+            use_spans.var_span_label_path_only(
                 &mut err,
                 format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()),
             );
@@ -255,6 +255,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                 partially_str,
                                 move_spans.describe()
                             ),
+                            "moved",
                         );
                     }
                 }
@@ -304,7 +305,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 }
             }
 
-            use_spans.var_span_label(
+            use_spans.var_span_label_path_only(
                 &mut err,
                 format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()),
             );
@@ -434,13 +435,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         err.span_label(borrow_span, format!("borrow of {} occurs here", borrow_msg));
         err.span_label(span, format!("move out of {} occurs here", value_msg));
 
-        borrow_spans.var_span_label(
+        borrow_spans.var_span_label_path_only(
             &mut err,
             format!("borrow occurs due to use{}", borrow_spans.describe()),
         );
 
-        move_spans
-            .var_span_label(&mut err, format!("move occurs due to use{}", move_spans.describe()));
+        move_spans.var_span_label(
+            &mut err,
+            format!("move occurs due to use{}", move_spans.describe()),
+            "moved",
+        );
 
         self.explain_why_borrow_contains_point(location, borrow, None)
             .add_explanation_to_diagnostic(
@@ -468,18 +472,24 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         let use_spans = self.move_spans(place.as_ref(), location);
         let span = use_spans.var_or_use();
 
+        // If the attempted use is in a closure then we do not care about the path span of the place we are currently trying to use
+        // we call `var_span_label` on `borrow_spans` to annotate if the existing borrow was in a closure
         let mut err = self.cannot_use_when_mutably_borrowed(
             span,
             &self.describe_any_place(place.as_ref()),
             borrow_span,
             &self.describe_any_place(borrow.borrowed_place.as_ref()),
         );
 
-        borrow_spans.var_span_label(&mut err, {
-            let place = &borrow.borrowed_place;
-            let desc_place = self.describe_any_place(place.as_ref());
-            format!("borrow occurs due to use of {}{}", desc_place, borrow_spans.describe())
-        });
+        borrow_spans.var_span_label(
+            &mut err,
+            {
+                let place = &borrow.borrowed_place;
+                let desc_place = self.describe_any_place(place.as_ref());
+                format!("borrow occurs due to use of {}{}", desc_place, borrow_spans.describe())
+            },
+            "mutable",
+        );
 
         self.explain_why_borrow_contains_point(location, borrow, None)
             .add_explanation_to_diagnostic(
@@ -591,6 +601,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                             desc_place,
                             borrow_spans.describe(),
                         ),
+                        "immutable",
                     );
 
                     return err;
@@ -667,7 +678,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         if issued_spans == borrow_spans {
             borrow_spans.var_span_label(
                 &mut err,
-                format!("borrows occur due to use of {}{}", desc_place, borrow_spans.describe()),
+                format!("borrows occur due to use of {}{}", desc_place, borrow_spans.describe(),),
+                gen_borrow_kind.describe_mutability(),
             );
         } else {
             let borrow_place = &issued_borrow.borrowed_place;
@@ -679,6 +691,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                     borrow_place_desc,
                     issued_spans.describe(),
                 ),
+                issued_borrow.kind.describe_mutability(),
             );
 
             borrow_spans.var_span_label(
@@ -688,6 +701,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                     desc_place,
                     borrow_spans.describe(),
                 ),
+                gen_borrow_kind.describe_mutability(),
             );
         }
 
@@ -847,7 +861,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             self.prefixes(borrow.borrowed_place.as_ref(), PrefixSet::All).last().unwrap();
 
         let borrow_spans = self.retrieve_borrow_spans(borrow);
-        let borrow_span = borrow_spans.var_or_use();
+        let borrow_span = borrow_spans.var_or_use_path_span();
 
         assert!(root_place.projection.is_empty());
         let proper_span = self.body.local_decls[root_place.local].source_info.span;
@@ -987,7 +1001,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             location, name, borrow, drop_span, borrow_spans
         );
 
-        let borrow_span = borrow_spans.var_or_use();
+        let borrow_span = borrow_spans.var_or_use_path_span();
         if let BorrowExplanation::MustBeValidFor {
             category,
             span,
@@ -1575,6 +1589,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 loan_spans.var_span_label(
                     &mut err,
                     format!("borrow occurs due to use{}", loan_spans.describe()),
+                    loan.kind.describe_mutability(),
                 );
 
                 err.buffer(&mut self.errors_buffer);
@@ -1585,8 +1600,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
 
         let mut err = self.cannot_assign_to_borrowed(span, loan_span, &descr_place);
 
-        loan_spans
-            .var_span_label(&mut err, format!("borrow occurs due to use{}", loan_spans.describe()));
+        loan_spans.var_span_label(
+            &mut err,
+            format!("borrow occurs due to use{}", loan_spans.describe()),
+            loan.kind.describe_mutability(),
+        );
 
         self.explain_why_borrow_contains_point(location, loan, None).add_explanation_to_diagnostic(
             self.infcx.tcx,

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

@@ -24,8 +24,8 @@ use super::{find_use, RegionName, UseSpans};
 
 #[derive(Debug)]
 pub(in crate::borrow_check) enum BorrowExplanation {
-    UsedLater(LaterUseKind, Span),
-    UsedLaterInLoop(LaterUseKind, Span),
+    UsedLater(LaterUseKind, Span, Option<Span>),
+    UsedLaterInLoop(LaterUseKind, Span, Option<Span>),
     UsedLaterWhenDropped {
         drop_loc: Location,
         dropped_local: Local,
@@ -67,22 +67,39 @@ impl BorrowExplanation {
         borrow_span: Option<Span>,
     ) {
         match *self {
-            BorrowExplanation::UsedLater(later_use_kind, var_or_use_span) => {
+            BorrowExplanation::UsedLater(later_use_kind, var_or_use_span, path_span) => {
                 let message = match later_use_kind {
                     LaterUseKind::TraitCapture => "captured here by trait object",
                     LaterUseKind::ClosureCapture => "captured here by closure",
                     LaterUseKind::Call => "used by call",
                     LaterUseKind::FakeLetRead => "stored here",
                     LaterUseKind::Other => "used here",
                 };
-                if !borrow_span.map_or(false, |sp| sp.overlaps(var_or_use_span)) {
-                    err.span_label(
-                        var_or_use_span,
-                        format!("{}borrow later {}", borrow_desc, message),
-                    );
+                // We can use `var_or_use_span` if either `path_span` is not present, or both spans are the same
+                if path_span.map(|path_span| path_span == var_or_use_span).unwrap_or(true) {
+                    if borrow_span.map(|sp| !sp.overlaps(var_or_use_span)).unwrap_or(true) {
+                        err.span_label(
+                            var_or_use_span,
+                            format!("{}borrow later {}", borrow_desc, message),
+                        );
+                    }
+                } else {
+                    // path_span must be `Some` as otherwise the if condition is true
+                    let path_span = path_span.unwrap();
+                    // path_span is only present in the case of closure capture
+                    assert!(matches!(later_use_kind, LaterUseKind::ClosureCapture));
+                    if !borrow_span.map_or(false, |sp| sp.overlaps(var_or_use_span)) {
+                        let path_label = "used here by closure";
+                        let capture_kind_label = message;
+                        err.span_label(
+                            var_or_use_span,
+                            format!("{}borrow later {}", borrow_desc, capture_kind_label),
+                        );
+                        err.span_label(path_span, path_label);
+                    }
                 }
             }
-            BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span) => {
+            BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span, path_span) => {
                 let message = match later_use_kind {
                     LaterUseKind::TraitCapture => {
                         "borrow captured here by trait object, in later iteration of loop"
@@ -94,7 +111,24 @@ impl BorrowExplanation {
                     LaterUseKind::FakeLetRead => "borrow later stored here",
                     LaterUseKind::Other => "borrow used here, in later iteration of loop",
                 };
-                err.span_label(var_or_use_span, format!("{}{}", borrow_desc, message));
+                // We can use `var_or_use_span` if either `path_span` is not present, or both spans are the same
+                if path_span.map(|path_span| path_span == var_or_use_span).unwrap_or(true) {
+                    err.span_label(var_or_use_span, format!("{}{}", borrow_desc, message));
+                } else {
+                    // path_span must be `Some` as otherwise the if condition is true
+                    let path_span = path_span.unwrap();
+                    // path_span is only present in the case of closure capture
+                    assert!(matches!(later_use_kind, LaterUseKind::ClosureCapture));
+                    if borrow_span.map(|sp| !sp.overlaps(var_or_use_span)).unwrap_or(true) {
+                        let path_label = "used here by closure";
+                        let capture_kind_label = message;
+                        err.span_label(
+                            var_or_use_span,
+                            format!("{}borrow later {}", borrow_desc, capture_kind_label),
+                        );
+                        err.span_label(path_span, path_label);
+                    }
+                }
             }
             BorrowExplanation::UsedLaterWhenDropped {
                 drop_loc,
@@ -311,13 +345,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 let borrow_location = location;
                 if self.is_use_in_later_iteration_of_loop(borrow_location, location) {
                     let later_use = self.later_use_kind(borrow, spans, location);
-                    BorrowExplanation::UsedLaterInLoop(later_use.0, later_use.1)
+                    BorrowExplanation::UsedLaterInLoop(later_use.0, later_use.1, later_use.2)
                 } else {
                     // Check if the location represents a `FakeRead`, and adapt the error
                     // message to the `FakeReadCause` it is from: in particular,
                     // the ones inserted in optimized `let var = <expr>` patterns.
                     let later_use = self.later_use_kind(borrow, spans, location);
-                    BorrowExplanation::UsedLater(later_use.0, later_use.1)
+                    BorrowExplanation::UsedLater(later_use.0, later_use.1, later_use.2)
                 }
             }
 
@@ -498,16 +532,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
     }
 
     /// Determine how the borrow was later used.
+    /// First span returned points to the location of the conflicting use
+    /// Second span if `Some` is returned in the case of closures and points
+    /// to the use of the path
     fn later_use_kind(
         &self,
         borrow: &BorrowData<'tcx>,
         use_spans: UseSpans<'tcx>,
         location: Location,
-    ) -> (LaterUseKind, Span) {
+    ) -> (LaterUseKind, Span, Option<Span>) {
         match use_spans {
-            UseSpans::ClosureUse { var_span, .. } => {
+            UseSpans::ClosureUse { capture_kind_span, path_span, .. } => {
                 // Used in a closure.
-                (LaterUseKind::ClosureCapture, var_span)
+                (LaterUseKind::ClosureCapture, capture_kind_span, Some(path_span))
             }
             UseSpans::PatUse(span)
             | UseSpans::OtherUse(span)
@@ -542,15 +579,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                 }
                             }
                         };
-                        return (LaterUseKind::Call, function_span);
+                        return (LaterUseKind::Call, function_span, None);
                     } else {
                         LaterUseKind::Other
                     }
                 } else {
                     LaterUseKind::Other
                 };
 
-                (kind, span)
+                (kind, span, None)
             }
         }
     }