suggestion for attempted integer identifier in patterns#106591
suggestion for attempted integer identifier in patterns#106591bors merged 1 commit intorust-lang:masterfrom
Conversation
|
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
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 |
tests/ui/pattern/issue-106552.stderr
Outdated
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
8 participants
Fixes #106552
Implemented a suggestion on
E0005that occurs when no bindings are present and the pattern is a literal integer.