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.

Sign up for GitHub

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

Jump to bottom

Reject negative literals for unsigned or char types in pattern ranges and literals #136304

Merged
merged 2 commits into from
Feb 6, 2025
Merged

Reject negative literals for unsigned or char types in pattern ranges and literals #136304

merged 2 commits into from
Feb 6, 2025
+75 −2

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jan 30, 2025 •
edited
Loading

It sucks a bit that we have to duplicate the work here (normal expressions just get this for free from the ExprKind::UnOp(UnOp::Neg, ...) typeck logic.

In #134228 I caused

fn main() {
    match 42_u8 {
        -10..255 => {},
        _ => {}
    }
}

to just compile without even a lint.

I can't believe we didn't have tests for this

Amusingly #136302 will also register a delayed bug in lit_to_const for this, so we'll have a redundancy if something like this fails again.

@rustbot
Copy link
Collaborator

rustbot commented Jan 30, 2025

r? @Nadrieril

rustbot has assigned @Nadrieril.
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 Jan 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 30, 2025

HIR ty lowering was modified

cc @fmease

@oli-obk oli-obk changed the title Reject negative literals for unsigned or char types Reject negative literals for unsigned or char types in pattern ranges and literals Jan 30, 2025
@oli-obk oli-obk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk oli-obk force-pushed the push-ymxoklvzrpvx branch 2 times, most recently from 6c69f93 to 91c5c59 Compare January 31, 2025 14:30
oli-obk
oli-obk commented Jan 31, 2025
View reviewed changes
tests/ui/type/pattern_types/signed_ranges.stderr
Comment on lines +24 to +5
error[E0277]: the trait bound `u8: Neg` is not satisfied
--> $DIR/signed_ranges.rs:14:9
|
LL | -10..253 => {}
| ^^^ the trait `Neg` is not implemented for `u8`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried improving this error by having it point to where we got the u8 type information from (the scrutinee), but adding the right obligations for that didn't work out, so I'll need to do some more debugging.

But this is also the exact same diagnostic we get on stable and beta, so...

@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 31, 2025

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 31, 2025
@Nadrieril
Copy link
Member

Nadrieril commented Feb 4, 2025 •
edited
Loading

Going back to the same error we had before is plenty enough.

@bors r+

@bors
Copy link
Contributor

bors commented Feb 4, 2025

📌 Commit 76ba91c has been approved by Nadrieril

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 Feb 4, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 4, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2025
@bors
Auto merge of rust-lang#136549 - matthiaskrgr:rollup-sqbpgtd, r=matth…
bef3c3b 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#136242 (Remove `LateContext::match_def_path()`)
 - rust-lang#136274 (Check Sizedness of return type in WF)
 - rust-lang#136284 (Allow using named consts in pattern types)
 - rust-lang#136477 (Fix a couple NLL TLS spans )
 - rust-lang#136497 (Report generic mismatches when calling bodyless trait functions)
 - rust-lang#136520 (Remove unnecessary layout assertions for object-safe receivers)
 - rust-lang#136526 (mir_build: Rename `thir::cx::Cx` to `ThirBuildCx` and remove `UserAnnotatedTyHelpers`)

Failed merges:

 - rust-lang#136304 (Reject negative literals for unsigned or char types in pattern ranges and literals)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Feb 5, 2025

☔ The latest upstream changes (presumably #136549) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 5, 2025
@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 5, 2025

@bors r=Nadrieril

we got the pattern types negation check for free from #136284

@bors
Copy link
Contributor

bors commented Feb 5, 2025

📌 Commit f44794f has been approved by Nadrieril

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 5, 2025
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Feb 5, 2025
@jieyouxu
Rollup merge of rust-lang#136304 - oli-obk:push-ymxoklvzrpvx, r=Nadri…
d940a36 
…eril

Reject negative literals for unsigned or char types in pattern ranges and literals

It sucks a bit that we have to duplicate the work here (normal expressions just get this for free from the `ExprKind::UnOp(UnOp::Neg, ...)` typeck logic.

In rust-lang#134228 I caused

```rust
fn main() {
    match 42_u8 {
        -10..255 => {},
        _ => {}
    }
}
```

to just compile without even a lint.

I can't believe we didn't have tests for this

Amusingly rust-lang#136302 will also register a delayed bug in `lit_to_const` for this, so we'll have a redundancy if something like this fails again.
@jieyouxu jieyouxu mentioned this pull request Feb 5, 2025
Closed
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Feb 5, 2025
@jieyouxu
Rollup merge of rust-lang#136304 - oli-obk:push-ymxoklvzrpvx, r=Nadri…
3647f7a 
…eril

Reject negative literals for unsigned or char types in pattern ranges and literals

It sucks a bit that we have to duplicate the work here (normal expressions just get this for free from the `ExprKind::UnOp(UnOp::Neg, ...)` typeck logic.

In rust-lang#134228 I caused

```rust
fn main() {
    match 42_u8 {
        -10..255 => {},
        _ => {}
    }
}
```

to just compile without even a lint.

I can't believe we didn't have tests for this

Amusingly rust-lang#136302 will also register a delayed bug in `lit_to_const` for this, so we'll have a redundancy if something like this fails again.
This was referenced Feb 5, 2025
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2025
@bors
Auto merge of rust-lang#136572 - jieyouxu:rollup-mtyaisw, r=jieyouxu
2d09365 
Rollup of 12 pull requests

Successful merges:

 - rust-lang#132547 (cg_gcc: Directly use rustc_abi instead of reexports)
 - rust-lang#135572 (tests: Port `split-debuginfo` to rmake.rs)
 - rust-lang#135964 (Make cenum_impl_drop_cast a hard error)
 - rust-lang#136154 (Use +secure-plt for powerpc-unknown-linux-gnu{,spe})
 - rust-lang#136304 (Reject negative literals for unsigned or char types in pattern ranges and literals)
 - rust-lang#136418 (uefi: process: Add support for command environment variables)
 - rust-lang#136449 (std: move network code into `sys`)
 - rust-lang#136517 (implement inherent str constructors)
 - rust-lang#136536 (Rename and Move some UI tests to more suitable subdirs)
 - rust-lang#136537 (Update `compiler-builtins` to 0.1.145)
 - rust-lang#136555 (Rename `slice::take...` methods to `split_off...`)
 - rust-lang#136567 (Arbitrary self types v2: recursion test)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2025
@bors
Auto merge of rust-lang#136572 - jieyouxu:rollup-mtyaisw, r=jieyouxu
6741521 
Rollup of 12 pull requests

Successful merges:

 - rust-lang#132547 (cg_gcc: Directly use rustc_abi instead of reexports)
 - rust-lang#135572 (tests: Port `split-debuginfo` to rmake.rs)
 - rust-lang#135964 (Make cenum_impl_drop_cast a hard error)
 - rust-lang#136154 (Use +secure-plt for powerpc-unknown-linux-gnu{,spe})
 - rust-lang#136304 (Reject negative literals for unsigned or char types in pattern ranges and literals)
 - rust-lang#136418 (uefi: process: Add support for command environment variables)
 - rust-lang#136449 (std: move network code into `sys`)
 - rust-lang#136517 (implement inherent str constructors)
 - rust-lang#136536 (Rename and Move some UI tests to more suitable subdirs)
 - rust-lang#136537 (Update `compiler-builtins` to 0.1.145)
 - rust-lang#136555 (Rename `slice::take...` methods to `split_off...`)
 - rust-lang#136567 (Arbitrary self types v2: recursion test)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f9f667f into rust-lang:master Feb 6, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 6, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2025
@rust-timer
Unrolled build for rust-lang#136304
6e84ab8 
Rollup merge of rust-lang#136304 - oli-obk:push-ymxoklvzrpvx, r=Nadrieril

Reject negative literals for unsigned or char types in pattern ranges and literals

It sucks a bit that we have to duplicate the work here (normal expressions just get this for free from the `ExprKind::UnOp(UnOp::Neg, ...)` typeck logic.

In rust-lang#134228 I caused

```rust
fn main() {
    match 42_u8 {
        -10..255 => {},
        _ => {}
    }
}
```

to just compile without even a lint.

I can't believe we didn't have tests for this

Amusingly rust-lang#136302 will also register a delayed bug in `lit_to_const` for this, so we'll have a redundancy if something like this fails again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Nadrieril Nadrieril

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
Milestone
1.86.0
Development

Successfully merging this pull request may close these issues.
5 participants
@oli-obk @rustbot @rust-log-analyzer @Nadrieril @bors