suggestion for attempted integer identifier in patterns #106591
Conversation
|
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
| a variable; identifiers cannot start with digits", | ||
| "_", | ||
| Applicability::MaybeIncorrect, | ||
| ); |
There was a problem hiding this comment.
Open to this, but it seems to me that I haven't really wanted to define an integer-named variable ~ever. Maybe instead of a help: text with _int we can just note: that identifiers cannot start with digits? I think in 99% of cases they probably wanted to use if let or similar, so adding a second suggestion may be more confusing.
At least to me if I see two suggestions I think I'd think "which is better?" before thinking "oh, I wanted an if, the second one is irrelevant".
There was a problem hiding this comment.
Obviously this is somewhat targeted at new developers but, personally, I've never let-matched on a integer literal before. I'd use a match statement instead which allows better handling of the default case, etc. FWIW, I think that if a user tries to do this (again, IMO very uncommon) then really meant to assign a value instead.
Ezrashaw
force-pushed
the
attempted-integer-identifer
branch
from
a94e510
to
b4be8b8
Compare
|
@Mark-Simulacrum I've rebased (moved the tests), depending on your review comment, it's good to go. |
|
☔ The latest upstream changes (presumably #106760) made this pull request unmergeable. Please resolve the merge conflicts. |
Ezrashaw
force-pushed
the
attempted-integer-identifer
branch
2 times, most recently
from
e6bec6a
to
63bd68f
Compare
rustbot
added
the
A-translation
|
cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki |
| error[E0005]: refutable pattern in local binding | ||
| --> $DIR/issue-106552.rs:5:9 | ||
| | | ||
| LL | let x @ 5 = 6; | ||
| | ^^^^^ patterns `i32::MIN..=4_i32` and `6_i32..=i32::MAX` not covered | ||
| | | ||
| = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant | ||
| = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html | ||
| = note: the matched value is of type `i32` | ||
| help: you might want to use `let else` to handle the variants that aren't matched | ||
| | | ||
| LL | let x @ 5 = 6 else { todo!() }; | ||
| | ++++++++++++++++ |
There was a problem hiding this comment.
Looking at this with fresh eyes, shouldn't we be suggesting if let here? 🤔
There was a problem hiding this comment.
I haven't touched the if let and else suggestion code, so I haven't really looked at this but: My understanding is that this situation is a bit like an alternative if let. You can bind the value, and if it doesn't match, the else branch is called (which must diverge).
There was a problem hiding this comment.
I'm not asking you to fix it now necessarily, just leaving a paper trail of my thinking so we don't forget to fix it in the future :)
|
r? @estebank r=me after addressing the nit picks |
Ezrashaw
force-pushed
the
attempted-integer-identifer
branch
from
63bd68f
to
1babece
Compare
|
@bors r=Estebank |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#106591 (suggestion for attempted integer identifier in patterns) - rust-lang#106712 (make error emitted on `impl &Trait` nicer) - rust-lang#106829 (Unify `Opaque`/`Projection` handling in region outlives code) - rust-lang#106869 (rustdoc: remove redundant item kind class from `.item-decl > pre`) - rust-lang#106949 (ConstBlocks are poly if their substs are poly) - rust-lang#106953 (Document `EarlyBinder::subst_identity` and `skip_binder`) - rust-lang#106958 (Don't add A-bootstrap to PRs modifying Cargo.lock) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #106552
Implemented a suggestion on
E0005that occurs when no bindings are present and the pattern is a literal integer.