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

False positive on while_let_on_iterator with nested while let #600

Closed
leoyvens opened this issue Jan 30, 2016 · 5 comments
Closed

False positive on while_let_on_iterator with nested while let #600

leoyvens opened this issue Jan 30, 2016 · 5 comments
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@leoyvens
Copy link

Clap exhibits what seems to be a false positive on while_let_on_iterator because it has a nested while let on the same iterator.

@llogiq
Copy link
Contributor

llogiq commented Jan 31, 2016

The error in question is:

src/app/parser.rs:447:9: 540:10 warning: this loop could be written as a `for` loop, #[warn(while_let_on_iterator)] on by default
src/app/parser.rs:447         while let Some(arg) = it.next() {
src/app/parser.rs:448             let arg_os = arg.into();
src/app/parser.rs:449             debugln!("Begin parsing '{:?}' ({:?})", arg_os, &*arg_os.as_bytes());
src/app/parser.rs:450 
src/app/parser.rs:451             // Is this a new argument, or values from a previous option?
src/app/parser.rs:452             debug!("Starts new arg...");
                      ...
src/app/parser.rs:447:9: 540:10 help: try
for arg in it {...}
for further information visit https://github.com/Manishearth/rust-clippy/wiki#while_let_on_iterator
src/app/parser.rs:517:21: 527:22 warning: this loop could be written as a `for` loop, #[warn(while_let_on_iterator)] on by default
src/app/parser.rs:517                     while let Some(v) = it.next() {
src/app/parser.rs:518                         let a = v.into();
src/app/parser.rs:519                         if let None = a.to_str() {
src/app/parser.rs:520                             if !self.settings.is_set(AppSettings::StrictUtf8)  {
src/app/parser.rs:521                                 return Err(
src/app/parser.rs:522                                     Error::invalid_utf8(&*self.create_current_usage(matcher))
                      ...
src/app/parser.rs:517:21: 527:22 help: try
for v in it {...}
for further information visit https://github.com/Manishearth/rust-clippy/wiki#while_let_on_iterator

The problem with trying to make either of those instances a for loop is that this would move the iterator into the loop. So I guess to fix this, we'll have to make sure that the iterator isn't used anywhere within the loop (and also mark used iterators so the inner loop will not be reported either).

@mcarton mcarton added the C-bug Category: Clippy is not doing the correct thing label Feb 13, 2016
@flying-sheep
Copy link
Contributor

jup, encountered this as well with this design.

i don’t like this at all but have no idea how to make it prettier. the idea is to find the pair of positions that points to the _n_th newline in both buffers b1 and b2, for the maximal n with n % 4 = 0

let mut b1it = b1.iter().enumerate();
let mut b2it = b2.iter().enumerate();

let mut chunk_ends = (0, 0);

loop {
    let mut new_chunk_end1 = 0; let mut c1 = 0;
    while let Some((pos, byte)) = b1it.next() {
        if *byte == b'\n' {
            c1 += 1;
            if c1 == 4 {
                new_chunk_end1 = pos + 1;
                break;
            }
        }
    }
    let mut new_chunk_end2 = 0; let mut c2 = 0;
    while let Some((pos, byte)) = b2it.next() {
        if *byte == b'\n' {
            c2 += 1;
            if c2 == 4 {
                new_chunk_end2 = pos + 1;
                break;
            }
        }
    }
    // if we found a whole new chunk in both
    if c1 == 4 && c2 == 4 {
        chunk_ends = (new_chunk_end1, new_chunk_end2);
    } else {
        break;
    }
}

do_stuff_with(&b1[0..chunk_ends.0], &b2[0..chunk_ends.1])

@oli-obk
Copy link
Contributor

oli-obk commented Feb 16, 2016

can't you just do a for loop that iterates over a &mut to the iterator? http://is.gd/b98D4M

@flying-sheep
Copy link
Contributor

ugh, true, yeah!

my code is still ugly there, but that part is prettier now 😄

@phansch
Copy link
Member

phansch commented Dec 18, 2020

This is not linted anymore: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a856a012f8a5c1564e9eb2c87caab9b7

We also have tests for nested loops:

fn nested_loops() {

I'm going to go ahead and close this issue.

@phansch phansch closed this as completed Dec 18, 2020
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
Projects
None yet
Development

No branches or pull requests

6 participants