Skip to content
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

Suggest ..= when someone tries to create an overflowing range #109554

Merged
merged 4 commits into from
Mar 30, 2023

Conversation

mu001999
Copy link
Contributor

Fixes #109529

@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2023

r? @oli-obk

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

@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 Mar 24, 2023
@mu001999
Copy link
Contributor Author

r? @scottmcm

@rustbot rustbot assigned scottmcm and unassigned oli-obk Mar 24, 2023
compiler/rustc_lint/src/types.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/types.rs Outdated Show resolved Hide resolved
@leonardo-m
Copy link

This suggestion seems OK, but keep in mind that using ..= has performance consequences (it often/sometimes gives slower code).

@mu001999 mu001999 requested a review from WaffleLapkin March 24, 2023 12:14
@WaffleLapkin WaffleLapkin self-assigned this Mar 24, 2023
@mu001999 mu001999 requested a review from WaffleLapkin March 24, 2023 17:03
@mu001999
Copy link
Contributor Author

mu001999 commented Mar 24, 2023

@WaffleLapkin Something weird is that the first symbol + is omitted if multi suggestions are also used for expressions with parentheses:

error: range endpoint is out of range for `u8`
  --> $DIR/deny-overflowing-literals.rs:5:14
   |
LL |     for _ in 0..256u8 {}
   |              ^^^^^^^^
   |
help: use an inclusive range instead
   |
LL |     for _ in 0..=255u8 {}
   |                 ~~~~~

I finally chose to emits suggestions separately. Luckily, the test results look good.

Please let me know if the behavior above is known or unexpected. Thanks! ;)

@scottmcm
Copy link
Member

scottmcm commented Mar 24, 2023

I'm not familiar enough with lint patterns to be a good reviewer here, so let's say

r? @WaffleLapkin

since they commented already.

EDIT: good job, self -- I should look at to whom it's actually assigned, not just read the r? comments 🤦

@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2023

Could not assign reviewer from: WaffleLapkin.
User(s) WaffleLapkin are either the PR author or are already assigned, and there are no other candidates.
Use r? to specify someone else to assign.

@scottmcm scottmcm removed their assignment Mar 24, 2023
@mu001999

This comment was marked as resolved.

tests/ui/lint/issue-109529.rs Show resolved Hide resolved
@WaffleLapkin
Copy link
Member

This looks fine to me, modulo // run-rustfix (I thought I added the comment ages ago but apparently no? anyway..,.)

@mu001999 mu001999 requested a review from WaffleLapkin March 29, 2023 02:07
@WaffleLapkin
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 29, 2023

📌 Commit dde26b3 has been approved by WaffleLapkin

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 Mar 29, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 29, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#107387 (Use random `HashMap` keys on Hermit)
 - rust-lang#109511 (Make `EvalCtxt`'s `infcx` private)
 - rust-lang#109554 (Suggest ..= when someone tries to create an overflowing range)
 - rust-lang#109675 (Do not consider elaborated projection predicates for objects in new solver)
 - rust-lang#109693 (Remove ~const from alloc)
 - rust-lang#109700 (Lint against escape sequences in Fluent files)
 - rust-lang#109716 (Move `mir::Field` → `abi::FieldIdx`)
 - rust-lang#109726 (rustdoc: Don't strip crate module)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 83573a3 into rust-lang:master Mar 30, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 30, 2023
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.

Suggest ..= when someone tries to create an overflowing range
7 participants