Resolve overflow behavior for RangeFrom

[CAD97]

This specifies a documented unspecified implementation detail of RangeFrom and makes it consistently implement the specified behavior.

Specifically, (u8::MAX).next() is defined to cause an overflow, and resolve that overflow in the same manner as the Step::forward implementation.

The inconsistency that has existed is <RangeFrom as Iterator>::nth. The existing behavior should be plain to see after #69659: the skipping part previously always panicked if it caused an overflow, but the final step (to set up the state for further iteration) has always been debug-checked.

The inconsistency, then, is that RangeFrom::nth does not implement the same behavior as the naive (and default) implementation of just calling next multiple times. This PR aligns RangeFrom::nth to have identical behavior to the naive implementation. It also lines up with the standard behavior of primitive math in Rust everywhere else in the language: debug checked overflow.

cc @Amanieu

---

Followup to #69659. Closes #25708 (by documenting the panic as intended).

The documentation wording is preliminary and can probably be improved.

This will probably need an FCP, as it changes observable stable behavior.

[rust-highfive]

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

r? @Amanieu

[Amanieu]

LGTM

@rfcbot fcp merge

[Amanieu]

@rfcbot fcp merge

[rfcbot]

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @SimonSapin
• @dtolnay
• @sfackler
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[SimonSapin]

It also lines up with the standard behavior of primitive math in Rust everywhere else in the language: debug checked overflow.

In most crates, rustc decides at compile-time whether to emit code for overflow checks based on configuration. The standard library however is distributed pre-compiled. I vaguely recall there is some mechanism to make standard library functions/methods run overflow checks based on whether the caller is in a crate that wants them. However I don’t remember any more than that, including how it’s called so I could grep for it.

How does this work? Does this require annotation to be added to impls of Iterator::nth, Step::forward and every other function that ends up on the call stack?

[Mark-Simulacrum]

It's the rustc_inherit_overflow_checks attribute. I believe it works on traits too. I think it only needs to be added to the function containing the + operator (or whatever math is being done).

[SimonSapin]

I believe it works on traits too.

You mean on trait impls, right? Like here:

rust/src/libcore/ops/arith.rs

Lines 89 to 95 in 64ad709

	impl Add for $t {
	type Output = $t;
	#[inline]
	#[rustc_inherit_overflow_checks]
	fn add(self, other: $t) -> $t { self + other }
	}

I think it only needs to be added to the function containing the + operator (or whatever math is being done).

@CAD97 Is that why src/libcore/iter/range.rs has calls to Add::add and Sub::sub in a couple places instead of using + and -? To try and take advantage of the rustc_inherit_overflow_checks attributes there?

---

rust/src/librustc_mir_build/hair/cx/mod.rs

Lines 69 to 72 in 64ad709

	// Some functions always have overflow checks enabled,
	// however, they may not get codegen'd, depending on
	// the settings for the crate they are codegened in.
	let mut check_overflow = attr::contains_name(attrs, sym::rustc_inherit_overflow_checks);

Per that comment, rustc_inherit_overflow_checks only works for generic code with a type parameter chosen by users of the standard library, so that code is translated when compiling these users rather than when pre-compiling the standard library. Is that correct?

src/libcore/iter/range.rs has macro-generated concrete impls such as impl Step for u32. Does not work there rustc_inherit_overflow_checks? Or does it rely on #[inline] attributes to only be translated downstream?

[Mark-Simulacrum]

I believe if something is marked as #[inline] it should work, but we'll probably want to check and document this somewhere -- maybe on the libs team page on forge (https://forge.rust-lang.org/libs/maintaining-std.html).

[SimonSapin]

My understanding of #[inline] is that it’s a hint that may or may not be observed by LLVM. Relying on it for documented semantics seems fragile.

[Mark-Simulacrum]

We don't need the function to be inlined by LLVM -- we just need the codegen to happen in leaf crates. Currently, that's the behavior that inline forces, so in that respect it should be fine. We rely on this for pretty much all of the "maybe checked" arithmetic exposed by libcore/std I think

[CAD97]

@SimonSapin

Is that why src/libcore/iter/range.rs has calls to Add::add and Sub::sub in a couple places instead of using + and -? To try and take advantage of the rustc_inherit_overflow_checks attributes there?

Yes, and this is the way that it was handled before the Step redesign touched everything in that file. Step::add_one was just #[inline] { Add::add(self, 1) }. Honestly, I'm not exactly certain how exactly inherited overflow checks work, I just know that they do.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[SimonSapin]

Sounds good, thanks all.

[dtolnay]

@bors r+

[bors]

📌 Commit 406852a has been approved by dtolnay

[RalfJung]

@bors rollup