Skip to content

while_let_on_iterator suggests broken fix when iterator is captured in FnMut #7249

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

Closed
zgotsch opened this issue May 19, 2021 · 0 comments · Fixed by #7262
Closed

while_let_on_iterator suggests broken fix when iterator is captured in FnMut #7249

zgotsch opened this issue May 19, 2021 · 0 comments · Fixed by #7262
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@zgotsch
Copy link

zgotsch commented May 19, 2021

I have noticed a similar problem to #5844 and #6231, where Clippy suggests a for loop instead of a while let, causing an illegal move of the target iterator. In my case, it was because the iterator was borrowed by reference in an FnMut closure.

Minimal example:

fn close_over(mut f: impl FnMut()) {
    f();
    f();
}

fn main() {
    let v = vec![1, 2, 3];
    let mut it = v.into_iter();
    close_over(|| {
        while let Some(x) = it.next() {
        // for x in it {
            println!("{}", x);
        }
    });
}

Rust Playground

Clippy suggests changing the while let Some(x) = it.next() to for x in it, but we cannot move out of the mutable reference to it. The correct suggestion is probably for x in &mut it, similar to fixes in #6966, but it seems more difficult to detect a FnMut without type information, which I'm not sure Clippy has access to. I believe the suggestion for x in it is better when the iterator has been moved into the closure.

Meta

  • cargo clippy -V: clippy 0.1.52 (88f19c6d 2021-05-03)
  • rustc -Vv:
rustc 1.52.0 (88f19c6da 2021-05-03)
binary: rustc
commit-hash: 88f19c6dab716c6281af7602e30f413e809c5974
commit-date: 2021-05-03
host: x86_64-apple-darwin
release: 1.52.0
LLVM version: 12.0.0
@zgotsch zgotsch added the C-bug Category: Clippy is not doing the correct thing label May 19, 2021
@giraffate giraffate added I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions labels May 20, 2021
bors added a commit that referenced this issue Jun 8, 2021
fix `while_let_on_iterator` suggestion in a closure

fixes: #7249

A future improvement would be to check if the closure is being used as `FnOnce`, in which case the original suggestion would be correct.

changelog: Suggest `&mut iter` inside a closure for `while_let_on_iterator`
@bors bors closed this as completed in 07217e3 Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants