Empty iterator error

[obeis]

Closes #100635

[rust-highfive]

r? @fee1-dead

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

[rust-log-analyzer]

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

---- [pretty] src/test/pretty/stmt_expr_attributes.rs stdout ----

error: pretty-printing failed in round 0 revision None
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/pretty/stmt_expr_attributes.rs" "-Z" "unpretty=normal" "--target" "x86_64-unknown-linux-gnu" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/pretty/stmt_expr_attributes/auxiliary.pretty" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers"
--- stdout -------------------------------
// pp-exact
#![feature(box_syntax)]
#![feature(inline_const)]
#![feature(inline_const_pat)]
#![feature(rustc_attrs)]
#![feature(rustc_attrs)]
#![feature(stmt_expr_attributes)]

fn main() {}

fn _0() {

    #[rustc_dummy]
    foo();

fn _1() {


    #[rustc_dummy]
    unsafe {
        #![rustc_dummy]
        // code
}

fn _2() {


    #[rustc_dummy]
    { foo(); }
    {
    {
        #![rustc_dummy]
        foo()
    }
}


fn _3() {

    #[rustc_dummy]
    match () { _ => {} }

fn _4() {


    #[rustc_dummy]
    match () {
        #![rustc_dummy]
        _ => (),

    let _ =
    let _ =
        #[rustc_dummy] match () {
            #![rustc_dummy]
            () => (),
}

fn _5() {


    #[rustc_dummy]
    let x = 1;

    let x = #[rustc_dummy] 1;
    let y = ();
    let y = ();
    let z = ();

    foo3(x, #[rustc_dummy] y, z);

    qux(3 + #[rustc_dummy] 2);

fn _6() {


    #[rustc_dummy]
    [1, 2, 3];

    let _ = #[rustc_dummy] [1, 2, 3];

    #[rustc_dummy]
    [1; 4];

    let _ = #[rustc_dummy] [1; 4];

struct Foo {
    data: (),
}
}

struct Bar(());

fn _7() {

    #[rustc_dummy]
    Foo { data: () };

    let _ = #[rustc_dummy] Foo { data: () };

fn _8() {


    #[rustc_dummy]
    ();

    #[rustc_dummy]
    (0);

    #[rustc_dummy]
    (0,);

    #[rustc_dummy]
    (0, 1);

fn _9() {
fn _9() {
    macro_rules! stmt_mac { () => { let _ = () ; } }

    #[rustc_dummy]
    stmt_mac!();

    #[rustc_dummy]
    stmt_mac! {};

    #[rustc_dummy]
    stmt_mac![];

    #[rustc_dummy]
    stmt_mac! {}
    let _ = ();
}


macro_rules! expr_mac { () => { () } }
fn _10() {
fn _10() {
    let _ = #[rustc_dummy] expr_mac!();
    let _ = #[rustc_dummy] expr_mac![];
    let _ = #[rustc_dummy] expr_mac! {};

fn _11() {
fn _11() {
    let _ = #[rustc_dummy] box 0;
    let _: [(); 0] = #[rustc_dummy] [];
    let _ = #[rustc_dummy] [0, 0];
    let _ = #[rustc_dummy] [0; 0];
    let _ = #[rustc_dummy] foo();
    let _ = #[rustc_dummy] 1i32.clone();
    let _ = #[rustc_dummy] ();
    let _ = #[rustc_dummy] (0);
    let _ = #[rustc_dummy] (0,);
    let _ = #[rustc_dummy] (0, 0);
    let _ = #[rustc_dummy] 0 + #[rustc_dummy] 0;
    let _ = #[rustc_dummy] !0;
    let _ = #[rustc_dummy] -0i32;
    let _ = #[rustc_dummy] false;
    let _ = #[rustc_dummy] 'c';
    let _ = #[rustc_dummy] 0;
    let _ = #[rustc_dummy] 0 as usize;
    let _ =
        #[rustc_dummy] while false {
            #![rustc_dummy]
    let _ =
    let _ =
        #[rustc_dummy] while let None = Some(()) {
            #![rustc_dummy]
    let _ =
    let _ =
        #[rustc_dummy] for _ in 0..0 {
            #![rustc_dummy]
    let _ =
    let _ =
        #[rustc_dummy] loop {
            #![rustc_dummy]
    let _ =
    let _ =
        #[rustc_dummy] match false {
            #![rustc_dummy]
            _ => (),
        };
    let _ = #[rustc_dummy] || #[rustc_dummy] ();
    let _ = #[rustc_dummy] move || #[rustc_dummy] ();
    let _ =
        #[rustc_dummy] ||
            {
                #![rustc_dummy]
                #[rustc_dummy]
            };
    let _ =
    let _ =
        #[rustc_dummy] move ||
            {
                #![rustc_dummy]
                #[rustc_dummy]
            };
    let _ =
    let _ =
        #[rustc_dummy] {
            #![rustc_dummy]
    let _ =
    let _ =
        #[rustc_dummy] {
            #![rustc_dummy]
            let _ = ();
    let _ =
    let _ =
        #[rustc_dummy] {
            #![rustc_dummy]
            let _ = ();
        };
    let const {
    let const {
                    #![rustc_dummy]
                } =
        #[rustc_dummy] const {
                #![rustc_dummy]
    let mut x = 0;
    let mut x = 0;
    let _ = #[rustc_dummy] x = 15;
    let _ = #[rustc_dummy] x += 15;
    let s = Foo { data: () };
    let _ = #[rustc_dummy] s.data;
    let _ = (#[rustc_dummy] s).data;
    let t = Bar(());
    let _ = #[rustc_dummy] t.0;
    let _ = (#[rustc_dummy] t).0;
    let v = vec!(0);
    let _ = #[rustc_dummy] v[0];
    let _ = (#[rustc_dummy] v)[0];
    let _ = #[rustc_dummy] 0..#[rustc_dummy] 0;
    let _ = #[rustc_dummy] 0..;
    let _ = #[rustc_dummy] (0..0);
    let _ = #[rustc_dummy] (0..);
    let _ = #[rustc_dummy] (..0);
    let _ = #[rustc_dummy] (..);
    let _: fn(&u32) -> u32 = #[rustc_dummy] std::clone::Clone::clone;
    let _ = #[rustc_dummy] &0;
    let _ = #[rustc_dummy] &mut 0;
    let _ = #[rustc_dummy] &#[rustc_dummy] 0;
    let _ = #[rustc_dummy] &mut #[rustc_dummy] 0;
    while false { let _ = #[rustc_dummy] continue; }
    while true { let _ = #[rustc_dummy] break; }
    || #[rustc_dummy] return;
    let _ = #[rustc_dummy] expr_mac!();
    let _ = #[rustc_dummy] expr_mac![];
    let _ = #[rustc_dummy] expr_mac! {};
    let _ = #[rustc_dummy] Foo { data: () };
    let _ = #[rustc_dummy] Foo { ..s };
    let _ = #[rustc_dummy] Foo { data: (), ..s };
    let _ = #[rustc_dummy] (0);

fn _12() {
fn _12() {
    #[rustc_dummy]
    let _ = 0;

    #[rustc_dummy]
    0;

    #[rustc_dummy]
    expr_mac!();

    #[rustc_dummy]
    {
        #![rustc_dummy]
}

fn foo() {}
fn foo() {}
fn foo3(_: i32, _: (), _: ()) {}
fn qux(_: i32) {}
--- stderr -------------------------------
--- stderr -------------------------------
error: empty iterator
    |
    |
169 |         #[rustc_dummy] for _ in 0..0 {

error: aborting due to previous error
------------------------------------------

[fee1-dead]

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

[WaffleLapkin]

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]

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

[rustbot]

rustc_error_messages was changed

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

[rust-log-analyzer]

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling core v0.0.0 (/checkout/library/core)
error: empty for iterator
   --> library/core/tests/iter/range.rs:464:5
    |
464 |     for _ in (10..0).rev() {
    |
    |
    = note: `-D empty-for-iterator` implied by `-D warnings`
error: empty for iterator
   --> library/core/tests/iter/range.rs:469:5
    |
    |
469 |     for _ in (10..0).rev() {

error: could not compile `core` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:19:24

[obeis]

@rustbot ready

[fee1-dead]

@rustbot author

[camsteffen]

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]

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?

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking rand v0.7.3
    Checking std v0.0.0 (/checkout/library/std)
    Checking alloc v0.0.0 (/checkout/library/alloc)
    Checking core v0.0.0 (/checkout/library/core)
error: unknown lint: `empty_iterator_range`
    |
    |
463 |     #![allow(empty_iterator_range)]
    |
    |
    = note: `-D unknown-lints` implied by `-D warnings`
error: could not compile `core` due to previous error
Build completed unsuccessfully in 0:01:47

[lcnr]

i think for _ in 10..0 could be a rustc lint but wouldn't expect rustc to maintain anything more complex. Having a lint which covers more cases is definitely good, though I would expect that one edit: to stay in clippy.

Idk whether we already have lints where rustc has a "stupid" version and clippy a more clever (and unstable/complex) one.

Don't have any strong feelings here though

[eddyb]

Quick notes:

• if you're matching on a complex source code shape, please consider a simpler one if relevant
  • here, IMO expr1..expr2 syntax (with the expressions evaluating to values that are out of order) seems like is the only relevant part? (not even iterator methods / IntoIterator::into_iter of a for loop)
• nowadays it seems safer to always prototype in clippy, and later "promote" to rustc
  • that is, as long as it's an "ergonomics" improvement (as opposed to dealing with some kind of soundness issue) and doesn't require hooking deep into an existing part of rustc
  • cc @rust-lang/clippy do we have a proper workflow for this? I would expect something like compiler team MCPs for uplifting from clippy to rustc (I could've sworn I've seen this discussed before but I'm not sure where to look now)
• there might be a neat way to give ourselves "permission" to keep expanding this analysis in a way that "range expressions" is insufficiently limiting:

#[clippy::weak_invariant(self.start < self.end)]
// alternative framing:
#[clippy::warn_if(self.start >= self.end)]
pub struct Range<T> {
    pub start: T,
    pub end: T,
}

That is, we would decorate standard library types (and functions, potentially) with "contract programming"-like annotations except there's no codegen (i.e. no way to generate runtime assertions from these annotations) and no strong compile-time enforcement (hence "weak" or "warn").

This would give us a general framework for "writing lints without writing lints", it would "excuse" global analysis (i.e. across function bodies), and would greatly benefit from e.g. using dataflow algorithms on MIR to acquire ranges/value-sets.

You know what else fits into all of those? Panicking. If you treat panicking as "warn if it happens" under this model, you can potentially find functions that pass constant arguments to other functions, causing unconditional panics (though being able to confirm "never panics" is of course more interesting than that "always panics").

(The "unconditional infinite recursion" lint, one of the few global analyses we have today, also fits, but it's less obvious how to model "the stack grows indefinitely", without ruling out "large but plausible/optimizable stack")

Also, reasoning about runtime values related to other runtime values should be possible (just not necessarily with the same "dataflow looking for constants" approaches), and that could be seen as an extension of this constant-only system. It could provide comparable power to some (less advanced) uses of e.g. dependent typing, but without changing the language (or rustc) to be able to represent these invariants (or pre/post-conditions when talking about functions) in the typesystem. At most, you could imagine that the attributes "enrich" the types/signatures they attach to, and then a separate type-{inference,check} algorithm runs, but even then that's kind of fuzzy and not really how it would be done (or at least not until you were seriously trying to get dependent typing and formal proofs).

The downside, of course, is that it's all "weak" and "warnings only" and so it cannot be relied on for safety. But it could reduce footguns in both safe (like this range thing) and unsafe APIs (NonZeroU32::new_unchecked(0) and MaybeUninit::uninit().assume_init() come to min). And at (ideally) much less cost than getting the same checking power for each situation individually.

[WaffleLapkin]

here, IMO expr1..expr2 syntax (with the expressions evaluating to values that are out of order) seems like is the only relevant part?

I don't think I agree, libraries can use ranges in ways, that make m..n (m >= n) sensible, so there is nothing wrong with m..n per-se, it's just that "RangeIter" (which is also Range which is very sad) with .start >= .end is empty which may not be expected.

cc https://github.com/orgs/rust-lang/teams/clippy do we have a proper workflow for this? I would expect something like compiler team MCPs for uplifting from clippy to rustc (I could've sworn I've seen this discussed before but I'm not sure where to look now)

Yes, according to https://github.com/rust-lang/rust/issues/53224#issuecomment-572809328 this needs approval from language and/or compiler team.

---

A more general way of warning on some conditions seems very interesting though 👀

[eddyb]

I don't think I agree, libraries can use ranges in ways, that make m..n (m >= n) sensible

rustc was doing something like that but it's a significant semantic mismatch and very easy to misuse, we ended up moving to a custom type anyway, in:

• #88242

https://github.com/rust-lang/rust/blob/187654481fd828e495919295369d33827f10e1c4/compiler/rustc_target/src/abi/mod.rs#L758-L773

[obeis]

@rustbot ready

[fee1-dead]

Probably needs consensus on whether we should uplift this or not.

r? rust-lang/compiler-team

[petrochenkov]

Nominated for the compiler team.

[petrochenkov]

I'm personally -0 on this, let it bake in clippy. But if it's uplifted, then it should be uplifted as is, without changes compared to the clippy version.

[apiraino]

discussed in T-compiler meeting on zulip, mostly on how to reconcile clippy and rustc lints, here are the (notes).

@rustbot label -I-compiler-nominated

[rustbot]

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]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking rand v0.7.3
    Checking std v0.0.0 (/checkout/library/std)
    Checking core v0.0.0 (/checkout/library/core)
    Checking alloc v0.0.0 (/checkout/library/alloc)
error: unknown lint: `empty_iterator_range`
    |
    |
463 |     #![allow(empty_iterator_range)]
    |
    |
    = note: `-D unknown-lints` implied by `-D warnings`
error: could not compile `core` due to previous error
Build completed unsuccessfully in 0:01:33

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking rand v0.7.3
    Checking alloc v0.0.0 (/checkout/library/alloc)
    Checking std v0.0.0 (/checkout/library/std)
    Checking core v0.0.0 (/checkout/library/core)
error: unknown lint: `empty_iterator_range`
    |
    |
463 |     #![allow(empty_iterator_range)]
    |
    |
    = note: `-D unknown-lints` implied by `-D warnings`
error: could not compile `core` due to previous error
Build completed unsuccessfully in 0:01:28

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking rand v0.7.3
    Checking std v0.0.0 (/checkout/library/std)
    Checking alloc v0.0.0 (/checkout/library/alloc)
    Checking core v0.0.0 (/checkout/library/core)
error: unknown lint: `empty_iterator_range`
    |
    |
463 |     #![cfg_attr(bootstrap, allow(empty_iterator_range))]
    |
    |
    = note: `-D unknown-lints` implied by `-D warnings`

error: unknown lint: `empty_iterator_range`
    |
    |
464 |     #![allow(empty_iterator_range)]

error: could not compile `core` due to 2 previous errors
Build completed unsuccessfully in 0:01:26

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking rand v0.7.3
    Checking alloc v0.0.0 (/checkout/library/alloc)
    Checking core v0.0.0 (/checkout/library/core)
    Checking std v0.0.0 (/checkout/library/std)
error: unknown lint: `empty_iterator_range`
    |
    |
464 |     #![allow(empty_iterator_range)]
    |
    |
    = note: `-D unknown-lints` implied by `-D warnings`
error: could not compile `core` due to previous error
Build completed unsuccessfully in 0:01:40

[bors]

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

[aDotInTheVoid]

@rustbot modify labels: -A-rustdoc-json

[apiraino]

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

[bors]

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