Auto merge of #133149 - estebank:niko-rustnation, r=wesleywiser

Provide more context on `Fn` closure modifying binding

When modifying a binding from inside of an `Fn` closure, point at the binding definition and suggest using an `std::sync` type that would allow the code to compile.

```
error[E0594]: cannot assign to `counter`, as it is a captured variable in a `Fn` closure
 --> f703.rs:6:9
  |
4 |     let mut counter = 0;
  |         ----------- `counter` declared here, outside the closure
5 |     let x: Box<dyn Fn()> = Box::new(|| {
  |                                     -- in this closure
6 |         counter += 1;
  |         ^^^^^^^^^^^^ cannot assign
  |
  = help: consider using `std::sync::atomic::AtomicI32` instead, which allows for multiple threads to access and modify the value
```

This provides more context on where the binding being modified was declared, and more importantly, guides newcomers towards `std::sync` when encountering cases like these.

When the requirement comes from an argument of a local function, we already tell the user that they might want to change from `Fn` to `FnMut`:

```
error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closure
  --> $DIR/borrow-immutable-upvar-mutation.rs:21:27
   |
LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
   |                                              - change this to accept `FnMut` instead of `Fn`
...
LL |         let mut x = 0;
   |             ----- `x` declared here, outside the closure
LL |         let _f = to_fn(|| x = 42);
   |                  ----- -- ^^^^^^ cannot assign
   |                  |     |
   |                  |     in this closure
   |                  expects `Fn` instead of `FnMut`
   |
   = help: consider using `std::sync::atomic::AtomicI32` instead, which allows for multiple threads to access and modify the value
```

We might want to avoid the `help` in this case.

_Inspired by a [part of Niko's keynote at RustNation UK 2024](https://youtu.be/04gTQmLETFI?si=dgJL2OJRtuShkxdD&t=600)._

Author: bors

compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

@@ -150,8 +150,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                     }
                 }
             }
-            PlaceRef { local: _, projection: [proj_base @ .., ProjectionElem::Deref] } => {
-                if the_place_err.local == ty::CAPTURE_STRUCT_LOCAL
+            PlaceRef { local, projection: [proj_base @ .., ProjectionElem::Deref] } => {
+                if local == ty::CAPTURE_STRUCT_LOCAL
                     && proj_base.is_empty()
                     && !self.upvars.is_empty()
                 {
@@ -165,10 +165,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         ", as `Fn` closures cannot mutate their captured variables".to_string()
                     }
                 } else {
-                    let source = self.borrowed_content_source(PlaceRef {
-                        local: the_place_err.local,
-                        projection: proj_base,
-                    });
+                    let source =
+                        self.borrowed_content_source(PlaceRef { local, projection: proj_base });
                     let pointer_type = source.describe_for_immutable_place(self.infcx.tcx);
                     opt_source = Some(source);
                     if let Some(desc) = self.describe_place(access_place.as_ref()) {
@@ -540,6 +538,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             PlaceRef { local, projection: [ProjectionElem::Deref] }
                 if local == ty::CAPTURE_STRUCT_LOCAL && !self.upvars.is_empty() =>
             {
+                self.point_at_binding_outside_closure(&mut err, local, access_place);
                 self.expected_fn_found_fn_mut_call(&mut err, span, act);
             }
 
@@ -958,6 +957,50 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         }
     }
 
+    /// When modifying a binding from inside of an `Fn` closure, point at the binding definition.
+    fn point_at_binding_outside_closure(
+        &self,
+        err: &mut Diag<'_>,
+        local: Local,
+        access_place: Place<'tcx>,
+    ) {
+        let place = access_place.as_ref();
+        for (index, elem) in place.projection.into_iter().enumerate() {
+            if let ProjectionElem::Deref = elem {
+                if index == 0 {
+                    if self.body.local_decls[local].is_ref_for_guard() {
+                        continue;
+                    }
+                    if let LocalInfo::StaticRef { .. } = *self.body.local_decls[local].local_info()
+                    {
+                        continue;
+                    }
+                }
+                if let Some(field) = self.is_upvar_field_projection(PlaceRef {
+                    local,
+                    projection: place.projection.split_at(index + 1).0,
+                }) {
+                    let var_index = field.index();
+                    let upvar = self.upvars[var_index];
+                    if let Some(hir_id) = upvar.info.capture_kind_expr_id {
+                        let node = self.infcx.tcx.hir_node(hir_id);
+                        if let hir::Node::Expr(expr) = node
+                            && let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind
+                            && let hir::def::Res::Local(hir_id) = path.res
+                            && let hir::Node::Pat(pat) = self.infcx.tcx.hir_node(hir_id)
+                        {
+                            let name = upvar.to_string(self.infcx.tcx);
+                            err.span_label(
+                                pat.span,
+                                format!("`{name}` declared here, outside the closure"),
+                            );
+                            break;
+                        }
+                    }
+                }
+            }
+        }
+    }
     /// Targeted error when encountering an `FnMut` closure where an `Fn` closure was expected.
     fn expected_fn_found_fn_mut_call(&self, err: &mut Diag<'_>, sp: Span, act: &str) {
         err.span_label(sp, format!("cannot {act}"));
@@ -970,6 +1013,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         let def_id = tcx.hir_enclosing_body_owner(fn_call_id);
         let mut look_at_return = true;
 
+        err.span_label(closure_span, "in this closure");
         // If the HIR node is a function or method call, get the DefId
         // of the callee function or method, the span, and args of the call expr
         let get_call_details = || {
@@ -1040,7 +1084,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             if let Some(span) = arg {
                 err.span_label(span, "change this to accept `FnMut` instead of `Fn`");
                 err.span_label(call_span, "expects `Fn` instead of `FnMut`");
-                err.span_label(closure_span, "in this closure");
                 look_at_return = false;
             }
         }
@@ -1067,7 +1110,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         sig.decl.output.span(),
                         "change this to return `FnMut` instead of `Fn`",
                     );
-                    err.span_label(closure_span, "in this closure");
                 }
                 _ => {}
             }

tests/ui/async-await/async-closures/wrong-fn-kind.stderr

@@ -25,6 +25,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
 LL | fn needs_async_fn(_: impl AsyncFn()) {}
    |                      -------------- change this to accept `FnMut` instead of `Fn`
 ...
+LL |     let mut x = 1;
+   |         ----- `x` declared here, outside the closure
 LL |     needs_async_fn(async || {
    |     -------------- ^^^^^^^^
    |     |              |

tests/ui/borrowck/borrow-immutable-upvar-mutation.stderr

@@ -4,6 +4,8 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
 LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
    |                                              - change this to accept `FnMut` instead of `Fn`
 ...
+LL |         let mut x = 0;
+   |             ----- `x` declared here, outside the closure
 LL |         let _f = to_fn(|| x = 42);
    |                  ----- -- ^^^^^^ cannot assign
    |                  |     |
@@ -16,6 +18,8 @@ error[E0596]: cannot borrow `y` as mutable, as it is a captured variable in a `F
 LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
    |                                              - change this to accept `FnMut` instead of `Fn`
 ...
+LL |         let mut y = 0;
+   |             ----- `y` declared here, outside the closure
 LL |         let _g = to_fn(|| set(&mut y));
    |                  ----- --     ^^^^^^ cannot borrow as mutable
    |                  |     |
@@ -28,6 +32,9 @@ error[E0594]: cannot assign to `z`, as it is a captured variable in a `Fn` closu
 LL | fn to_fn<A: std::marker::Tuple, F: Fn<A>>(f: F) -> F {
    |                                              - change this to accept `FnMut` instead of `Fn`
 ...
+LL |         let mut z = 0;
+   |             ----- `z` declared here, outside the closure
+...
 LL |             to_fn(|| z = 42);
    |             ----- -- ^^^^^^ cannot assign
    |             |     |

tests/ui/borrowck/borrow-raw-address-of-mutability.stderr

@@ -40,6 +40,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
 LL | fn make_fn<F: Fn()>(f: F) -> F { f }
    |                        - change this to accept `FnMut` instead of `Fn`
 ...
+LL |     let mut x = 0;
+   |         ----- `x` declared here, outside the closure
 LL |     let f = make_fn(|| {
    |             ------- -- in this closure
    |             |

tests/ui/borrowck/mutability-errors.stderr

@@ -139,7 +139,9 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
    |
 LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
    |                       - change this to accept `FnMut` instead of `Fn`
-...
+LL |
+LL | fn ref_closure(mut x: (i32,)) {
+   |                ----- `x` declared here, outside the closure
 LL |     fn_ref(|| {
    |     ------ -- in this closure
    |     |
@@ -152,7 +154,9 @@ error[E0594]: cannot assign to `x.0`, as `Fn` closures cannot mutate their captu
    |
 LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
    |                       - change this to accept `FnMut` instead of `Fn`
-...
+LL |
+LL | fn ref_closure(mut x: (i32,)) {
+   |                ----- `x` declared here, outside the closure
 LL |     fn_ref(|| {
    |     ------ -- in this closure
    |     |
@@ -166,7 +170,9 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
    |
 LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
    |                       - change this to accept `FnMut` instead of `Fn`
-...
+LL |
+LL | fn ref_closure(mut x: (i32,)) {
+   |                ----- `x` declared here, outside the closure
 LL |     fn_ref(|| {
    |     ------ -- in this closure
    |     |
@@ -180,7 +186,9 @@ error[E0596]: cannot borrow `x.0` as mutable, as `Fn` closures cannot mutate the
    |
 LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
    |                       - change this to accept `FnMut` instead of `Fn`
-...
+LL |
+LL | fn ref_closure(mut x: (i32,)) {
+   |                ----- `x` declared here, outside the closure
 LL |     fn_ref(|| {
    |     ------ -- in this closure
    |     |

tests/ui/closures/aliasability-violation-with-closure-21600.stderr

@@ -4,6 +4,9 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
 LL | fn call_it<F>(f: F) where F: Fn() { f(); }
    |                  - change this to accept `FnMut` instead of `Fn`
 ...
+LL |     let mut x = A;
+   |         ----- `x` declared here, outside the closure
+...
 LL |         call_it(|| x.gen_mut());
    |         ------- -- ^ cannot borrow as mutable
    |         |       |
@@ -16,6 +19,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
 LL | fn call_it<F>(f: F) where F: Fn() { f(); }
    |                  - change this to accept `FnMut` instead of `Fn`
 ...
+LL |     let mut x = A;
+   |         ----- `x` declared here, outside the closure
 LL |     call_it(|| {
    |     ------- -- in this closure
    |     |

tests/ui/closures/wrong-closure-arg-suggestion-125325.stderr

@@ -4,6 +4,8 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
 LL |     fn assoc_func(&self, _f: impl Fn()) -> usize {
    |                              --------- change this to accept `FnMut` instead of `Fn`
 ...
+LL |     let mut x = ();
+   |         ----- `x` declared here, outside the closure
 LL |     s.assoc_func(|| x = ());
    |       --------------^^^^^^-
    |       |          |  |
@@ -17,6 +19,9 @@ error[E0594]: cannot assign to `x`, as it is a captured variable in a `Fn` closu
 LL | fn func(_f: impl Fn()) -> usize {
    |             --------- change this to accept `FnMut` instead of `Fn`
 ...
+LL |     let mut x = ();
+   |         ----- `x` declared here, outside the closure
+...
 LL |     func(|| x = ())
    |     ---- -- ^^^^^^ cannot assign
    |     |    |

tests/ui/nll/closure-captures.stderr

@@ -47,7 +47,9 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
    |
 LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
    |                       - change this to accept `FnMut` instead of `Fn`
-...
+LL |
+LL | fn two_closures_ref_mut(mut x: i32) {
+   |                         ----- `x` declared here, outside the closure
 LL |     fn_ref(|| {
    |     ------ -- in this closure
    |     |
@@ -89,6 +91,8 @@ error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `F
 LL | fn fn_ref<F: Fn()>(f: F) -> F { f }
    |                       - change this to accept `FnMut` instead of `Fn`
 ...
+LL | fn two_closures_ref(x: i32) {
+   |                     - `x` declared here, outside the closure
 LL |     fn_ref(|| {
    |     ------ -- in this closure
    |     |

tests/ui/unboxed-closures/unboxed-closures-mutated-upvar-from-fn-closure.stderr

@@ -4,6 +4,8 @@ error[E0594]: cannot assign to `counter`, as it is a captured variable in a `Fn`
 LL | fn call<F>(f: F) where F : Fn() {
    |               - change this to accept `FnMut` instead of `Fn`
 ...
+LL |     let mut counter = 0;
+   |         ----------- `counter` declared here, outside the closure
 LL |     call(|| {
    |     ---- -- in this closure
    |     |