by estebank | 2025-12-10 19:18:08+00:00
I take umbrage at "The error that teaches nothing". I went to reproduce the case[1] in order to file a diagnostics ticket, and I don't know how you could make this diagnostic any more pedagogical:

error[E0382]: use of moved value: `results` --> src/main.rs:30:22 | 22 | let results = Arc::new(Mutex::new(Vec::new())); | ------- move occurs because `results` has type `Arc<std::sync::Mutex<Vec<i32>>>`, which does not implement the `Copy` trait ... 28 | for i in 0..20 { | -------------- inside of this loop 29 | // let results = Arc::clone(&results); 30 | pool.execute(move || { | ^^^^^^^ value moved into closure here, in previous iteration of loop 31 | let mut r = results.lock().unwrap(); | ------- use occurs due to use in closure | help: consider moving the expression out of the loop so it is only moved once | 28 ~ let mut value = results.lock(); 29 ~ for i in 0..20 { 30 | // let results = Arc::clone(&results); 31 | pool.execute(move || { 32 ~ let mut r = value.unwrap(); | help: consider cloning the value before moving it into the closure | 30 ~ let value = results.clone(); 31 ~ pool.execute(move || { 32 ~ let mut r = value.lock().unwrap(); |

> The compiler catches this. Good. But the fix isn't "add a mutex" - you already have a mutex.

The compiler doesn't tell you to "add a mutex".

Trying the first suggestion blindly does lead to a sadly very verbose output in an effort to try to explain all the moving parts of the API:

error[E0277]: `std::sync::MutexGuard<'_, Vec<i32>>` cannot be sent between threads safely --> src/main.rs:30:22 | 30 | pool.execute(move || { | ------- ^------ | | | | ______________|_______within this `{closure@src/main.rs:30:22: 30:29}` | | | | | required by a bound introduced by this call 31 | | let mut r = results.unwrap(); 32 | | r.push(i); 33 | | }); | |_________^ `std::sync::MutexGuard<'_, Vec<i32>>` cannot be sent between threads safely | = help: within `{closure@src/main.rs:30:22: 30:29}`, the trait `Send` is not implemented for `std::sync::MutexGuard<'_, Vec<i32>>` note: required because it appears within the type `Result<std::sync::MutexGuard<'_, Vec<i32>>, PoisonError<std::sync::MutexGuard<'_, Vec<i32>>>>` --> /playground/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/result.rs:550:10 | 550 | pub enum Result<T, E> { | ^^^^^^ note: required because it's used within this closure --> src/main.rs:30:22 | 30 | pool.execute(move || { | ^^^^^^^ note: required by a bound in `ThreadPool::execute` --> src/main.rs:15:23 | 13 | fn execute<F>(&self, f: F) | ------- required by a bound in this associated function 14 | where 15 | F: FnOnce() + Send + 'static, | ^^^^ required by this bound in `ThreadPool::execute`

The code that produces that first suggestion does have this comment I left there a year ago (because it was meant to help with a very different case)[2], so clearly I suspected the logic to be not completely correct back then, but likely not realized just how much so:

// FIXME: We could check that the call's *parent* takes `&mut val` to make the // suggestion more targeted to the `mk_iter(val).next()` case. Maybe do that only to // check for whether to suggest `let value` or `let mut value`.

I just created a ticket to better gate that suggestion so it doesn't trigger here[3].

But the second suggestion is exactly what the user intended. The only thing missing would be for the message to actually mention "that Arc::clone() creates a new owned handle that can be moved into the closure independently" as part of the message. That gives makes me think we should have something like `#[diagnostic::on_move("message")]`, so I filed a ticket for that too[4]. At worst we could hardcode better messages for std types.

I am not impressed with the rest of the article, neither the tone nor the substance. I wouldn't have commented if it hadn't been due to the combination of the title with that specific wording (given that I've spent a decade making rustc errors teach you things), the actionable feedback being buried under layers of snark and indirection, and the actual feedback being given being wrong (but hey, at least I did find two other tangential things that could do with fixing, so small wins I guess?).

[1]: https://play.rust-lang.org/?version=stable&mode=debug&editio...

[2]: https://github.com/rust-lang/rust/pull/121652

[3]: https://github.com/rust-lang/rust/issues/149861

[4]: https://github.com/rust-lang/rust/issues/149862

Edit: found the logic error. for loops desugar to a bare loop with a break, and the selection logic didn't account for that. https://github.com/rust-lang/rust/pull/149863