4 ms·
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 t
by estebank 10mo ago
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&edition=2024&gist=2811eaa6a23df876f239ac9aa4b23825 https://play.rust-lang.org/?version=stable&mode=debug&editio...
[2]: https://github.com/rust-lang/rust/pull/121652 https://github.com/rust-lang/rust/pull/121652
[3]: https://github.com/rust-lang/rust/issues/149861 https://github.com/rust-lang/rust/issues/149861
[4]: https://github.com/rust-lang/rust/issues/149862 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 https://github.com/rust-lang/rust/pull/149863