Skip to content

Add regression test for saturating_sub bounds check issue #145064

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

Merged
merged 1 commit into from
Aug 10, 2025

Conversation

okaneco
Copy link
Contributor

@okaneco okaneco commented Aug 7, 2025

Add codegen test for issue where valid_index.saturating_sub(X) produced an extra bounds check.
This was fixed by the LLVM upgrade.

Closes #139759

@rustbot
Copy link
Collaborator

rustbot commented Aug 7, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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

@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. labels Aug 7, 2025
@okaneco okaneco force-pushed the saturating_sub_regression_tests branch from 77e7ee3 to 9647d03 Compare August 7, 2025 18:58
Add codegen test for issue where `valid_index.saturating_sub(X)` produced an
extra bounds check.
This was fixed by the LLVM upgrade.
@okaneco okaneco force-pushed the saturating_sub_regression_tests branch from 9647d03 to 163594c Compare August 7, 2025 19:12
Copy link
Member

@xizheyin xizheyin left a comment

Choose a reason for hiding this comment

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

Based on experience, I think using test cases with suffixes such as *-issue-139759.rs seems to be more common (whether to change it or not is up to you). In any case, everything else is fine.

@nikic
Copy link
Contributor

nikic commented Aug 10, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 10, 2025

📌 Commit 163594c has been approved by nikic

It is now in the queue for this repository.

@bors bors 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 Aug 10, 2025
bors added a commit that referenced this pull request Aug 10, 2025
Rollup of 7 pull requests

Successful merges:

 - #144553 (Rehome 32 `tests/ui/issues/` tests to other subdirectories under `tests/ui/`)
 - #145064 (Add regression test for `saturating_sub` bounds check issue)
 - #145121 (bootstrap: `x.py dist rustc-src` should keep LLVM's siphash)
 - #145150 (Replace unsafe `security_attributes` function with safe `inherit_handle` alternative)
 - #145152 (Use `eq_ignore_ascii_case` to avoid heap alloc in `detect_confuse_type`)
 - #145200 (mbe: Fix typo in attribute tracing)
 - #145222 (Fix typo with paren rustc_llvm/build.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a7a4e17 into rust-lang:master Aug 10, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 10, 2025
@okaneco okaneco deleted the saturating_sub_regression_tests branch August 10, 2025 23:44
rust-timer added a commit that referenced this pull request Aug 10, 2025
Rollup merge of #145064 - okaneco:saturating_sub_regression_tests, r=nikic

Add regression test for `saturating_sub` bounds check issue

Add codegen test for issue where `valid_index.saturating_sub(X)` produced an extra bounds check.
This was fixed by the LLVM upgrade.

Closes #139759
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bounds check not elided for index that should be known to be in-bounds
7 participants