check_match: misc unifications and ICE fixes #68399
Conversation
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ |
|
📌 Commit 58eb03d has been approved by |
bors
added
S-waiting-on-bors
|
cc @rust-lang/wg-diagnostics |
check_match: misc unifications and ICE fixes These are some unifications made as a by-product of working on `hir::ExprKind::Let`. Fixes rust-lang#68396. Fixes rust-lang#68394. Fixes rust-lang#68393. r? @oli-obk @matthewjasper
Rollup of 7 pull requests Successful merges: - #67686 (Simplify NodeHeader by avoiding slices in BTreeMaps with shared roots) - #68140 (Implement `?const` opt-out for trait bounds) - #68313 (Options IP_MULTICAST_TTL and IP_MULTICAST_LOOP are 1 byte on BSD) - #68328 (Actually pass target LLVM args to LLVM) - #68399 (check_match: misc unifications and ICE fixes) - #68415 (tidy: fix most clippy warnings) - #68416 (lowering: cleanup some hofs) Failed merges: r? @ghost
| @@ -0,0 +1,9 @@ | |||
| error[E0080]: runtime values cannot be referenced in patterns | |||
There was a problem hiding this comment.
I would like the wording of this error to be tweaked a bit.
There was a problem hiding this comment.
What's wrong with the current?
There was a problem hiding this comment.
I feel it is a bit jargony, but I guess it's no worse than other more common errors we emit.
| error[E0158]: associated consts cannot be referenced in patterns | ||
| --> $DIR/issue-68393-let-pat-assoc-constant.rs:20:40 | ||
| | | ||
| LL | pub fn test<A: Foo, B: Foo>(arg: EFoo, A::X: EFoo) { | ||
| | ^^^^ | ||
|
|
||
| error[E0158]: associated consts cannot be referenced in patterns | ||
| --> $DIR/issue-68393-let-pat-assoc-constant.rs:22:9 | ||
| | | ||
| LL | let A::X = arg; | ||
| | ^^^^ |
There was a problem hiding this comment.
Can we customize the "patterns" wording in fn args and let bindings? I would expect people that are unfamiliar with the language to get confused by this. At least a note with something like (but better) "let statements introduce new bindings to some pattern, normally a simple name which are also patterns".
There was a problem hiding this comment.
I think this is unlikely that a beginner would write this code personally. Using an associated constant in an irrefutable context seems fairly niche. In fact, I found this ICE by trying to unify the code, realizing that a check was missing in the irrefutable contexts. A::X also does not seem confusable for a binding. I would prefer not to preempt better diagnostics here until a beginner shows up.
If we are going to improve diagnostics, I'd rather add "defined here" span_labels for X and A.
| //~^ ERROR variable `Foo` should have a snake case name | ||
| //~^^ WARN `Foo` is named the same as one of the variants of the type `foo::Foo` | ||
| //~^^^ WARN unused variable: `Foo` |
There was a problem hiding this comment.
Can we use //~| in this file?
There was a problem hiding this comment.
I mostly copied existing code. Should we file an issue for follow-up?
|
☔ The latest upstream changes (presumably #68423) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
These are some unifications made as a by-product of working on
hir::ExprKind::Let.Fixes #68396.
Fixes #68394.
Fixes #68393.
r? @oli-obk @matthewjasper