Provide more context on Fn closure modifying binding

[estebank]

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.

[rustbot]

r? @cjgillot

rustbot has assigned @cjgillot.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[compiler-errors]

I'm somewhat worried that this will lead to users trying to mutate state through APIs that are designed to be immutable. Taking an impl Fn() can have a lot of different intentions from an API designer perspective, and there's not much context we can glean by simply just seeing a mutate-within-Fn error in borrowck. Mentioning threading and atomics unprompted seems a bit strange, even though it could fix some people's errors, and even if it does, it's possibly not the right solution compared to using a different API or approaching a problem differently.

I'd prefer if this help message were elaborated to mention that the function expects a Fn and to tell the user pay attention to why this is the case, and then advising the user that if mutation is intended then it can be done concurrently through an atomic since it does not require exclusive access.

I fear that if we provide just the information in this help message, we're at risk of misleading users further by turning borrowck errors into logical or design errors that they cannot so easily debug or catch.

[compiler-errors]

@rustbot author

[estebank]

@compiler-errors what do you think if instead of specifying the type they can use we instead linked at the book section on shared state? I did not make it a structured suggestion to introduce a bit of friction to people blindly following this, but your point stands.