Lint against iterator functions that panics when N is zero

[Urgau]

This PR adds new kind of variant to the deny-by-default unconditional_panic lint, by linting on iterator functions that panics when N (chunks/windows size) is zero.

error: this operation will panic at runtime
  --> $DIR/const-n-is-zero.rs:11:13
   |
LL |     let _ = s.array_windows::<0>();
   |             ^^^^^^^^^^^^^^^^^^^^^^ const parameter `N` is zero
   |
   = note: `#[deny(unconditional_panic)]` on by default

cc @rust-lang/libs-api

[rustbot]

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @vakaras

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

[rustbot]

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 69 candidates
• Random selection from 16 candidates

[RalfJung]

This PR adds new kind of variant to the deny-by-default unconditional_panic lint, by linting on iterator functions that panics when N (chunks/windows size) is zero.

This seems to do much more than that. It adds a new language primitive, a new kind of built-in assertion. Could you motivate that?

[Urgau]

This seems to do much more than that. It adds a new language primitive, a new kind of built-in assertion. Could you motivate that?

The code is unfortunately tangled, it doesn't add a new language primitive per se, but report_assert_as_lint requires a AssertKind variant even if it's just for linting in one place.

I could probably modify that function so it doesn't take an AssertKind, but I wasn't sure about that.

[RalfJung]

Given that AssertKind is part of the MIR syntax, I feel very strongly that we shouldn't modify it for non-MIR/opsem purposes.

[Urgau]

Understandable, I've reworked the PR to avoid touching at AssertKind.

[JohnTitor]

I love the idea but it should better for someone more familiar with this topic other than me to review. @RalfJung Could you take over? Otherwise I'd reroll.

View changes since this review

[RalfJung]

I'm afraid not, I don't have capacity at the moment.

@rustbot reroll

[fee1-dead]

I think the overall idea is good and I could see myself approving down the line, however I think we should see if we can have an attribute on the generic parameter itself instead of applying rustc_panics_when_n_is_zero to the enclosing function. If not, I'd still want something more verbose, such as #[rustc_panics_when_const_param_is_zero(N)]

I don't think this should be touching AssertLint. I feel like it might be simpler to just add a separate error for this rather than using existing mechanisms.

View changes since this review

[Urgau]

I had to add the encoding of const-params in rmeta, but otherwise I've moved the attribute directly to the const generic.

I've also removed any interaction with AssertLint, but kept the reporting behind the unconditional_panic lint.

@rustbot ready

[fee1-dead]

overall looks good, would like someone else to sign off with me on this together though

@rustbot reroll

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.