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

Fix suggestion of explicit_counter_loop #4691

Merged
merged 3 commits into from
Oct 23, 2019
Merged

Conversation

HMPerson1
Copy link
Contributor

@HMPerson1 HMPerson1 commented Oct 18, 2019

changelog: In the suggestion of explicit_counter_loop, if the for loop argument doesn't implement Iterator, then we suggest x.into_iter().enumerate() (or x.iter{_mut}() as appropriate). Also, the span of the suggestion has been corrected.

Fixes #4678

@HMPerson1
Copy link
Contributor Author

I just noticed that the suggestion is kinda wrong; we suggest replacing just the argument of the for loop with an entire for [new_pat] in [new_arg].
To produce the correct suggestion, we would need to somehow get the span of just for [] in [] (without the loop body). Is that doable?

@HMPerson1
Copy link
Contributor Author

Okay it looks like the only thing that retains the span of the entire original for loop is the loop {..} expr in the body of the arm of the outermost match:

https://github.com/rust-lang/rust/blob/fa0f7d0080d8e7e9eb20aa9cbf8013f96c81287f/src/librustc/hir/lowering/expr.rs#L1167

{
    let result = match ::std::iter::IntoIterator::into_iter(<head>) {
        mut iter => {
            [opt_ident]: loop { // <------------- this expr
                let mut __next;
                match ::std::iter::Iterator::next(&mut iter) {
                    ::std::option::Option::Some(val) => __next = val,
                    ::std::option::Option::None => break
                };
                let <pat> = __next;
                StmtKind::Expr(<body>);
            }
        }
    };
    result
}

@HMPerson1 HMPerson1 changed the title Suggest calling iter if needed in explicit_counter_loop Fix suggestion of explicit_counter_loop Oct 18, 2019
@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 18, 2019
@phansch
Copy link
Member

phansch commented Oct 23, 2019

@bors r+ thanks!

@bors
Copy link
Collaborator

bors commented Oct 23, 2019

📌 Commit a9cb2b9 has been approved by phansch

bors added a commit that referenced this pull request Oct 23, 2019
Fix suggestion of `explicit_counter_loop`

changelog: In the suggestion of `explicit_counter_loop`, if the `for` loop argument doesn't implement `Iterator`, then we suggest `x.into_iter().enumerate()` (or `x.iter{_mut}()` as appropriate). Also, the span of the suggestion has been corrected.

Fixes #4678
@bors
Copy link
Collaborator

bors commented Oct 23, 2019

⌛ Testing commit a9cb2b9 with merge 087e5ea...

@bors
Copy link
Collaborator

bors commented Oct 23, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: phansch
Pushing 087e5ea to master...

@bors bors merged commit a9cb2b9 into rust-lang:master Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

explicit_counter_loop suggests enumerate when it does not exist
4 participants