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

iter_with_drain false positive when no ownership of the vector #8538

Closed
dtolnay opened this issue Mar 15, 2022 · 5 comments · Fixed by #8668
Closed

iter_with_drain false positive when no ownership of the vector #8538

dtolnay opened this issue Mar 15, 2022 · 5 comments · Fixed by #8668
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@dtolnay
Copy link
Member

dtolnay commented Mar 15, 2022

Summary

The iter_with_drain lint says to replace vec.drain(..) with vec.into_iter(). It sounds like what this has in mind is the impl<T> IntoIterator for Vec<T>, but only the owner of the vec could call that, which does not work in general for all uses of drain(..).

Replacing vec.drain(..) with vec.into_iter() when vec is a mutable reference ends up calling impl<T> IntoIterator for &mut Vec<T> which has a different element type than drain(..), so that usually wouldn't work. Also, clippy would immediately trigger into_iter_on_ref in that case, telling you to remove the .into_iter() in favor of .iter_mut().

Lint Name

iter_with_drain

Reproducer

pub struct Struct(Vec<String>);

pub fn repro(vec: &mut Vec<String>) -> Struct {
    Struct(vec
        .drain(..)
        .filter(|_i| /* ... */ true)
        .collect())
}
warning: `drain(..)` used on a `Vec`
 --> src/main.rs:5:10
  |
5 |         .drain(..)
  |          ^^^^^^^^^ help: try this: `into_iter()`
  |
  = note: `#[warn(clippy::iter_with_drain)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain

Following the suggestion causes an error.

error[E0277]: a value of type `Vec<String>` cannot be built from an iterator over elements of type `&mut String`
    --> src/main.rs:4:12
     |
4    |       Struct(vec
     |  _____------_^
     | |     |
     | |     required by a bound introduced by this call
5    | |         .into_iter()
6    | |         .filter(|_i| /* ... */ true)
7    | |         .collect())
     | |__________________^ value of type `Vec<String>` cannot be built from `std::iter::Iterator<Item=&mut String>`

Version

rustc 1.61.0-nightly (285fa7ecd 2022-03-14)
binary: rustc
commit-hash: 285fa7ecd05dcbfdaf2faaf20400f5f92b39b3c6
commit-date: 2022-03-14
host: x86_64-unknown-linux-gnu
release: 1.61.0-nightly
LLVM version: 14.0.0

Additional Labels

@rustbot label +I-suggestion-causes-error

@dtolnay dtolnay added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Mar 15, 2022
@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Mar 15, 2022
dtolnay added a commit to dtolnay/ghost that referenced this issue Mar 15, 2022
rust-lang/rust-clippy#8538

    warning: `drain(..)` used on a `Vec`
      --> src/variance.rs:32:10
       |
    32 |         .drain(..)
       |          ^^^^^^^^^ help: try this: `into_iter()`
       |
       = note: `#[warn(clippy::iter_with_drain)]` on by default
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain
@dtolnay
Copy link
Member Author

dtolnay commented Mar 15, 2022

Mentioning @ldm0 @flip1995 @giraffate since the lint is from #8483.

bors added a commit that referenced this issue Mar 15, 2022
Move iter_with_drain to nursery

cc #8538 #8539

r? `@giraffate`

changelog: Move [`iter_with_drain`] to nursery
MingweiSamuel added a commit to MingweiSamuel/hydroflow that referenced this issue Mar 16, 2022
@MingweiSamuel
Copy link
Contributor

This also applies even if you have ownership of the Vec, but still need to keep it after draining. For example

fn example(mut vec: Vec<u32>) -> Vec<u32> {
   vec.drain(..).for_each(|x| println!("{}", x);
   vec
}

MingweiSamuel added a commit to MingweiSamuel/hydroflow that referenced this issue Mar 16, 2022
@Bromeon
Copy link

Bromeon commented Mar 16, 2022

Can confirm, we experienced in godot-rust with a &mut Vec parameter (the error appeared in a CI run of clippy nightly).

@ViliamVadocz
Copy link

This issue should be reopened because clippy is still giving false positives.

Reproducible example:

fn main() {
    let mut my_vec = Vec::new();
    for i in 0..10 {
        my_vec.push(i);
        for x in my_vec.drain(..) {
            println!("{x}");
        }
    }
}

False positive:

warning: `drain(..)` used on a `Vec`
   --> bug/src/main.rs:5:25
    |
  5 |         for x in my_vec.drain(..) {
    |                         ^^^^^^^^^ help: try: `into_iter()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain

Clippy version: clippy 0.1.82 (2cbbe8b8 2024-07-28)

@Jarcho
Copy link
Contributor

Jarcho commented Jul 30, 2024

That's a distinctly different issue due to conflicting borrows. A new issue should be opened if one already isn't.

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-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants