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

Moving structs with Copy fields into closures causes surprising behavior #111376

Open
alexblanck opened this issue May 9, 2023 · 3 comments
Open
Labels
A-closures Area: Closures (`|…| { … }`) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alexblanck
Copy link

alexblanck commented May 9, 2023

I noticed some counterintuitive behavior related to partial moves of Copy struct fields (ex: u64 fields) into closures.

Minimal Reproducible Example

#[derive(Debug)]
struct Statistics {
    calls: u64,
    successes: u64,
}

fn main() {
    let mut statistics = Statistics {
        calls: 0,
        successes: 0,
    };
    
    let mut generator = move || {
        statistics.calls += 1;
    };
    
    generator();
    statistics.successes += 1;

    println!("{statistics:?}");
}

This code compiles and prints

Statistics { calls: 0, successes: 1 }

Playground Link

What I Expected

I would have expected a borrow of partially moved value: 'statistics' error at the println!, since the closure takes ownership of the statistics.calls field and then the field is later read by the print call.

The actual behavior, where the updates to statistics.calls have no observable effect, looks just like a silent copy of the whole struct. This surprised me. When I wrote the code, I expected either a compiler error or an output of Statistics { calls: 1, successes: 1 }.

Investigation

I experimented a bit with the types of the fields within the struct, and I do get the compiler error I expect if Statistics.calls is a non-Copy type like Vec.

I didn't know about partial moves when I wrote the initial code. I expected the whole struct to move into the closure. It's still surprising to me that partial moves can create this type of "silent copy" behavior depending on the type of the struct's fields.

I did find a similar issue titled Move closure copies :Copy variables silently, but the code that I wrote is not covered by the "unused variable" warning added in response to that issue.

Solution

I'm new to Rust so I'm not sure this is a bug, but it's still confusing behavior. Perhaps, like the other issue I linked, there is room for a lint or warning here. Something that warned me that the closure's modification of statistics.calls had no effect would be helpful, for example.

@alexblanck alexblanck added the C-bug Category: This is a bug. label May 9, 2023
@cuviper
Copy link
Member

cuviper commented May 10, 2023

This is a disjoint capture specific to 2021 edition. If you change your playground to 2018, you do get an error, though not exactly what you expected. The closure is only capturing the calls field, and move makes that a copy just for that u64.

@alexblanck
Copy link
Author

alexblanck commented May 15, 2023

Thanks for the link about disjoint captures. That's roughly what I gathered was happening, but I didn't know that term.

I still feel like a warning saying that the change to the captured, copied field had no effect could be helpful in this case.

@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-closures Area: Closures (`|…| { … }`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage-legacy labels Sep 7, 2023
@fmease
Copy link
Member

fmease commented Sep 7, 2023

Related to (but still distinct from) #73467.

@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: Closures (`|…| { … }`) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. 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

4 participants