Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misleading error when a value moved into a closure gets depleted (moved away) when that closure is called #133881

Open
mcclure opened this issue Dec 5, 2024 · 0 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mcclure
Copy link

mcclure commented Dec 5, 2024

Code

Check out this repo:
https://github.com/mcclure/rs-bug/tree/z_bug_clone_error
branch z_bug_clone_error.

There are two adjacent commits. 2b22470 exhibits the bad error, be44a0022 fixes the bad error (by adding a .clone().

To reproduce, you must run cargo build --example vim.

This is a somewhat heaviweight example. I attempted to produce a simpler example, and it actually produced a subtly different (better actually, but still confusing) error. So I guess I now have two examples. Here's example #2.

fn main() {
    #[derive(Debug, Clone)]
    struct Garbage {
        a: i32,
        b: i32
    }

    let garbage = Garbage { a:2, b:3 };

    let mut next_value = move || {
        let garbage2 = garbage;
        println!("{garbage2:?}");
    };

    while true {
        next_value();
    }

    println!("Hello, world!");
}

Current output

Example #1 ("z_bug_clone_error"):

error[E0525]: expected a closure that implements the `FnMut` trait, but this closure only implements `FnOnce`
    --> examples/vim.rs:1108:26
     |
1108 |     let mut next_value = move || {
     |                          ^^^^^^^ this closure implements `FnOnce`, not `FnMut`
...
1146 |             state.adjust = reset_adjust; // Implicit reset each loop. Consider making customizable?
     |                            ------------ closure is `FnOnce` because it moves the variable `reset_adjust` out of its environment
...
1204 |                     &mut next_value
     |                     --------------- the requirement to implement `FnMut` derives from here
     |
     = note: required for the cast from `&mut {closure@examples/vim.rs:1108:26: 1108:33}` to `&mut dyn FnMut() -> f32`

Example #2 (inline in post above)

error[E0382]: use of moved value: `next_value`
  --> src/main.rs:16:9
   |
16 |         next_value();
   |         ^^^^^^^^^^--
   |         |
   |         `next_value` moved due to this call, in previous iteration of loop
   |
note: closure cannot be invoked more than once because it moves the variable `garbage` out of its environment
  --> src/main.rs:11:24
   |
11 |         let garbage2 = garbage;
   |                        ^^^^^^^
note: this value implements `FnOnce`, which causes it to be moved when called
  --> src/main.rs:16:9
   |
16 |         next_value();
   |         ^^^^^^^^^^

Rationale and extra context

In both code examples, the same process is happening: There is a move || closure, the closure captures a variable, the closure does something that causes the variable to get consumed/moved out, the closure gets called more than once. The solution is to either call the closure only once, or to clone the value inside the closure rather than moving it.

Desired output

I have a couple problems with these errors.

The biggest problem is both errors describe my closure as "implementing" FnOnce or FnMut. Of course I don't implement any traits, I'm relying on builtin syntax to create the closure. I would understand the sentence "you've written a closure that can only be called once, but you call it more than once" but both errors word it "you implemented FnOnce instead of FnMut". This is technically correct, but it is phrasing the problem in terms of compiler concepts which are here hidden from the user.

The second problem is exclusive to error #1, and it is that the error describes the problem as a problem with the type of the closure whereas the actual issue is a problem with how next_value gets called or a problem with how reset_adjust/garbage is being treated. This is not made clear in the current wording. I don't understand why the two bits of very similar code produce different errors.

I am also a little confused by the wording "moves the variable reset_adjust out of its environment". In retrospect, I'm not sure it's reasonable I was confused, but I'll do my best: I have a move closure which moves the reset_adjust value out of its original environment (the stack frame from which it is captured). Then the line state.adjust = reset_adjust moves it out of its new environment and into state.adjust (although it doesn't move very far, because state.adjust also lives inside the move ||; maybe this is my fault for not understanding the meaning of "environment", but that kinda looks like the same environment to me). Because there are multiple moves and multiple different possible "environment"s in play, "the variable is moved out of its environment" does not guide me to the correct point in the code.

That's a lot of typing and I never said what my desired output would be. I don't actually know how you shold fix this error. But I think my recommendation would be to make the error look more like "example #2" than "example #1", and add another hint to indicate that the problem can be solved by cloning (or, in general, by making a change to reset_adjust/garbage). Earlier I commented there are two possible fixes: Fix the usage of next_value outside the closure, or fix the usage of reset_adjust/garbage inside the closure. The "Example #2" error above pushes you toward the first solution, but neither error pushes me toward the second.

I did get this error "in the wild", and it was slow for me to figure out the fix even though the fix is simple.

Rust Version

rustc 1.83.0 (90b35a623 2024-11-26)
binary: rustc
commit-hash: 90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf
commit-date: 2024-11-26
host: x86_64-unknown-linux-gnu
release: 1.83.0
LLVM version: 19.1.1

Anything else?

No response

@mcclure mcclure added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 5, 2024
@mcclure mcclure changed the title Misleading error when a mut value moved into a closure gets depleted (moved away) in a call Misleading error when a value moved into a closure gets depleted (moved away) when that closure is called Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

1 participant