rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.63k stars 12.48k forks source link

`impl Trait` changes mutability requirements of closure #60711

Open nikonthethird opened 5 years ago

nikonthethird commented 5 years ago

This code behaves in a way that is not obvious to me:

fn main() {
    fn get_impl_trait() -> impl Default {}
    let impl_trait = get_impl_trait();

    let mut unit = ();

    let modify_unit = || {
        let _captured_impl_trait = impl_trait; // (1)
        unit = (); // (2)
    };

    modify_unit();
}

This code compiles without any warnings in stable Rust. But notice, the modify_unit closure is not declared mut, but it modifies the unit variable inside at location (2).

However, when you comment out the line marked (1), suddenly Rust complains that the closure is not mutable:

Link to the code on the playground.

error[E0596]: cannot borrow `modify_unit` as mutable, as it is not declared as mutable
  --> src/main.rs:13:5
   |
7  |     let modify_unit = || {
   |         ----------- help: consider changing this to be mutable: `mut modify_unit`
...
13 |     modify_unit();
   |     ^^^^^^^^^^^ cannot borrow as mutable

What is the reason for this behavior?

jonas-schievink commented 5 years ago

In the first case, modify_unit is actually a move closure since it has to capture impl_trait by value (it isn't Copy). You can test this by trying to call it again. Since calling a closure via FnOnce consumes the closure instead of mutating it, this is accepted.

In the second case, no by-value capturing is needed (instead, only unit is captured by mutable reference), so calling the closure would AFAIK default to using FnMut, which takes &mut self and thus needs the variable to be mutable.

jonas-schievink commented 5 years ago

Perhaps the "cannot borrow as mutable" diagnostics could be improved in general to note the precise reason for the mutable borrow (since there are plenty of implicit ways to borrow something)

nikonthethird commented 5 years ago

Thank you for pointing that out, it didn't occur to me at all!