Skip to content

refactor RangeFromIter overflow-checks impl#154191

Open
pitaj wants to merge 1 commit intorust-lang:mainfrom
pitaj:fix-154124
Open

refactor RangeFromIter overflow-checks impl#154191
pitaj wants to merge 1 commit intorust-lang:mainfrom
pitaj:fix-154124

Conversation

@pitaj
Copy link
Contributor

@pitaj pitaj commented Mar 21, 2026

Crates with different overflow-checks settings accessing the same RangeFromIter resulted in incorrect values being yielded

Fixes #154124

r? @tgross35

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 21, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 21, 2026

tgross35 is currently at their maximum review capacity.
They may take a while to respond.

Comment on lines +27 to +40

// let a_next_ocno = next_ocno(&mut a.clone());
// let a_next_ocyes = next_ocyes(&mut a);
// let b_next_ocyes = next_ocyes(&mut b.clone());
// let b_next_ocno = next_ocno(&mut b);

// assert_eq!(n, a_next_ocno);
// assert_eq!(a_next_ocno, a_next_ocyes);
// assert_eq!(n, b_next_ocyes);
// assert_eq!(b_next_ocyes, b_next_ocno);


// assert_eq!(next_ocno(&mut a.clone()), next_ocyes(&mut a));
// assert_eq!(next_ocyes(&mut b.clone()), next_ocno(&mut b));
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Knew I was going to forget about this 🤦‍♂️

Comment on lines +43 to +50
// `b` should have wrapped
assert_eq!(0, next_ocyes(&mut b.clone()));
assert_eq!(0, next_ocno(&mut b));
// `a` should be exhausted,
// which will wrap when called without overflow-checks
assert_eq!(0, next_ocno(&mut a.clone()));
// and panic when called with overflow-checks
let r = std::panic::catch_unwind(move || {
Copy link
Contributor

Choose a reason for hiding this comment

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

(optional nit) Maybe give a and b names like next_with_oc and next_without_oc? It's a bit tricky to follow

@tgross35
Copy link
Contributor

Checking what I said in #154191 (comment)

@bors try jobs=x86_64-gnu-nopt

Cc issue author @theemathas

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 22, 2026

⌛ Trying commit e173b53 with merge e81a45d

To cancel the try build, run the command @bors try cancel.

Workflow: https://github.com/rust-lang/rust/actions/runs/23408886027

rust-bors bot pushed a commit that referenced this pull request Mar 22, 2026
refactor RangeFromIter overflow-checks impl


try-job: x86_64-gnu-nopt
@tgross35
Copy link
Contributor

@bors try cancel

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 22, 2026

Try build cancelled. Cancelled workflows:

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

r=me with the two items that weren't resolved by me rereading test headers

View changes since this review

Crates with different overflow-checks settings accessing the same RangeFromIter resulted in incorrect values being yielded
@tgross35
Copy link
Contributor

@bors r+

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 22, 2026

📌 Commit 8befc9d has been approved by tgross35

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RangeFromIter gives wrong values when mixing crates with different overflow-checks settings

3 participants