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

Empty iterator error #100751

Closed
wants to merge 3 commits into from
Closed

Empty iterator error #100751

wants to merge 3 commits into from

Conversation

obeis
Copy link
Contributor

@obeis obeis commented Aug 19, 2022

Closes #100635

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 19, 2022
@rust-highfive
Copy link
Collaborator

r? @fee1-dead

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2022
Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for the contribution! Could you please write this as a lint? (See rustc-dev-guide) You could also take a look at how clippy implemented this.

@rust-log-analyzer

This comment has been minimized.

@fee1-dead
Copy link
Member

fee1-dead commented Aug 19, 2022

I'm available on Zulip if you need any help.

@WaffleLapkin
Copy link
Member

If done as a late lint I think this could also evaluate constants, for example

const A: u32 = 10;
const B: u32 = 0;

fn main() {
    for i in A..B {
        println!("{i}");
    }
}

@fee1-dead
Copy link
Member

@rustbot author marking as waiting on author to address comments.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 23, 2022
@rustbot
Copy link
Collaborator

rustbot commented Aug 24, 2022

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

@rustbot rustbot added the A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic label Aug 24, 2022
@rust-log-analyzer

This comment has been minimized.

@obeis
Copy link
Contributor Author

obeis commented Aug 24, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 24, 2022
src/test/ui/lint/empty-for-iterator.rs Outdated Show resolved Hide resolved
library/core/tests/iter/range.rs Outdated Show resolved Hide resolved
compiler/rustc_error_messages/locales/en-US/lint.ftl Outdated Show resolved Hide resolved
compiler/rustc_lint/src/empty_iterator.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/empty_iterator.rs Outdated Show resolved Hide resolved
@fee1-dead
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 25, 2022
library/core/tests/iter/range.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/empty_iterator.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/empty_iterator.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/empty_iterator.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/empty_iterator.rs Outdated Show resolved Hide resolved
@camsteffen
Copy link
Contributor

camsteffen commented Aug 25, 2022

This is a lint with a lot of room for enhancement. Static value range analysis could be used to reveal empty ranges x..y. Also there are ways other than for-loops to misuse an empty range (e.g. (2..1).map(|x| ..)). So that makes me wonder if this is really a good candidate for a rustc lint rather than space for 3rd party tools to explore.

@fee1-dead
Copy link
Member

I was also wondering about this. Clippy has the lint.

However I think it would be nice to just add the for _ in 10..0 part and retain the other additional checks in clippy.

@rust-lang/compiler opinions?

@rustbot rustbot removed the I-compiler-nominated Nominated for discussion during a compiler team meeting. label Sep 15, 2022
@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-rustdoc-json Area: Rustdoc JSON backend labels Sep 17, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 17, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@obeis obeis force-pushed the issue-100635 branch 2 times, most recently from 5fca1a9 to 25580e3 Compare September 17, 2022 22:19
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Oct 4, 2022

☔ The latest upstream changes (presumably #102652) made this pull request unmergeable. Please resolve the merge conflicts.

@aDotInTheVoid
Copy link
Member

@rustbot modify labels: -A-rustdoc-json

@rustbot rustbot removed the A-rustdoc-json Area: Rustdoc JSON backend label Oct 10, 2022
@apiraino
Copy link
Contributor

apiraino commented Oct 13, 2022

Visiting again in T-compiler meeting on Zulip (notes). The general comment is that this lint should probably live in clippy in the first place rather than being implemented in the compiler.

@rustbot label -S-waiting-on-team

@rustbot rustbot removed the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Oct 18, 2022
@bors
Copy link
Contributor

bors commented Nov 24, 2022

☔ The latest upstream changes (presumably #104809) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 24, 2022
@Dylan-DPC
Copy link
Member

@obeis any updates on this?

@fee1-dead
Copy link
Member

Closing as this should live in clippy for now.

@fee1-dead fee1-dead closed this Jan 24, 2023
@obeis obeis deleted the issue-100635 branch May 13, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest valid reverse for loop if an invalid variant is used