Point at unit structs on foreign crates in type errors when they are the pattern of a binding#153894
Conversation
…the pattern of a let binding Consts and unit structs in patterns can be confusing if they are mistaken for new bindings. We already provide some context for unit structs and consts that come from the current crate, we now also point at those from foreign crates, and we properly skip cases where the pattern has type parameters which can't be confused with a new binding. Make new binding suggestion verbose.
|
r? @nnethercote rustbot has assigned @nnethercote. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
The example from #129792 seems reasonable: The use of a lower-case The example from this PR's description seems much less likely. Do we really think the problem in that code is that the user intended |
|
Agreed. The second commit uses the prior existence of suggestions as a signal that we shouldn't talk about introducing the new binding. The only case left in the test suite (other than the one in the ticket) is and Which I feel is the exact case we want to address, right? The only other thing I'd consider is making it so that in the cases where we're suggesting changing the case, we don't include the |
| help: introduce a new binding instead | ||
| | | ||
| LL - BSTR_SIZED => {} | ||
| LL + other_bstr_sized => {} |
There was a problem hiding this comment.
This one feels marginal, and likewise the other ones in this test. I can imagine a user thinking this would match against b"012". But it's sufficiently broken thinking that it's hard to say for sure, so I can live with it.
There was a problem hiding this comment.
I'm currently looking at a completely different case, and maybe the suggestion I'm adding might also be relevant here?
error: constant of non-structural type `S` in a pattern
--> $DIR/match_ice.rs:11:9
|
LL | struct S;
| -------- `S` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
...
LL | const C: &S = &S;
| ----------- constant defined here
LL | match C {
LL | C => {}
| ^ constant of non-structural type
|
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
help: if `S` manually implemented `PartialEq`, you could check for equality
|
LL - C => {}
LL + binding if binding == C => {}
|
Note that the binding suggestion is pre-existing, it's just being made more obvious now.
|
@bors r+ rollup |
Point at unit structs on foreign crates in type errors when they are the pattern of a binding Consts and unit structs in patterns can be confusing if they are mistaken for new bindings. We already provide some context for unit structs and consts that come from the current crate, we now also point at those from foreign crates, and we properly skip cases where the pattern has type parameters which can't be confused with a new binding. So not suggest making a new binding when other suggestions are already emitted, as the likelihood of the other suggestions being what the user intended is higher. Make new binding suggestion verbose. Fix rust-lang#129792. ``` error[E0308]: mismatched types --> fi.rs:8:9 | 1 | struct percentage; | ----------------- unit struct defined here ... 8 | let percentage = 4i32; | ^^^^^^^^^^ ---- this expression has type `i32` | | | expected `i32`, found `percentage` | `percentage` is interpreted as a unit struct, not a new binding help: introduce a new binding instead | 8 - let percentage = 4i32; 8 + let other_percentage = 4i32; | ```
|
⌛ Testing commit 408066f with merge 501c798... Workflow: https://github.com/rust-lang/rust/actions/runs/23190523244 |
Point at unit structs on foreign crates in type errors when they are the pattern of a binding Consts and unit structs in patterns can be confusing if they are mistaken for new bindings. We already provide some context for unit structs and consts that come from the current crate, we now also point at those from foreign crates, and we properly skip cases where the pattern has type parameters which can't be confused with a new binding. So not suggest making a new binding when other suggestions are already emitted, as the likelihood of the other suggestions being what the user intended is higher. Make new binding suggestion verbose. Fix #129792. ``` error[E0308]: mismatched types --> fi.rs:8:9 | 1 | struct percentage; | ----------------- unit struct defined here ... 8 | let percentage = 4i32; | ^^^^^^^^^^ ---- this expression has type `i32` | | | expected `i32`, found `percentage` | `percentage` is interpreted as a unit struct, not a new binding help: introduce a new binding instead | 8 - let percentage = 4i32; 8 + let other_percentage = 4i32; | ```
|
Stuck |
|
Auto build was cancelled. Cancelled workflows: The next pull request likely to be tested is #154001. |
Point at unit structs on foreign crates in type errors when they are the pattern of a binding Consts and unit structs in patterns can be confusing if they are mistaken for new bindings. We already provide some context for unit structs and consts that come from the current crate, we now also point at those from foreign crates, and we properly skip cases where the pattern has type parameters which can't be confused with a new binding. So not suggest making a new binding when other suggestions are already emitted, as the likelihood of the other suggestions being what the user intended is higher. Make new binding suggestion verbose. Fix rust-lang#129792. ``` error[E0308]: mismatched types --> fi.rs:8:9 | 1 | struct percentage; | ----------------- unit struct defined here ... 8 | let percentage = 4i32; | ^^^^^^^^^^ ---- this expression has type `i32` | | | expected `i32`, found `percentage` | `percentage` is interpreted as a unit struct, not a new binding help: introduce a new binding instead | 8 - let percentage = 4i32; 8 + let other_percentage = 4i32; | ```
…uwer Rollup of 14 pull requests Successful merges: - #153972 (stdarch subtree update) - #153801 (Add the option to run UI tests with the parallel frontend) - #153959 (Fix non-module `parent_module` in stripped cfg diagnostics) - #153967 (Tweak wording of failed predicate in inference error) - #152968 (Flip "region lattice" in RegionKind doc comment) - #153531 (Fix LegacyKeyValueFormat report from docker build: various) - #153622 (remove concept of soft-unstable features) - #153709 (Fix hypothetical ICE in `variances_of`) - #153884 (test `classify-runtime-const` for `f16`) - #153894 (Point at unit structs on foreign crates in type errors when they are the pattern of a binding) - #153920 (improve `#[track_caller]` invalid ABI error) - #153946 (dissolve `tests/ui/cross`) - #153965 (Fix minor kasan bugs) - #153991 (Small report_cycle refactor)
…uwer Rollup of 14 pull requests Successful merges: - rust-lang/rust#153972 (stdarch subtree update) - rust-lang/rust#153801 (Add the option to run UI tests with the parallel frontend) - rust-lang/rust#153959 (Fix non-module `parent_module` in stripped cfg diagnostics) - rust-lang/rust#153967 (Tweak wording of failed predicate in inference error) - rust-lang/rust#152968 (Flip "region lattice" in RegionKind doc comment) - rust-lang/rust#153531 (Fix LegacyKeyValueFormat report from docker build: various) - rust-lang/rust#153622 (remove concept of soft-unstable features) - rust-lang/rust#153709 (Fix hypothetical ICE in `variances_of`) - rust-lang/rust#153884 (test `classify-runtime-const` for `f16`) - rust-lang/rust#153894 (Point at unit structs on foreign crates in type errors when they are the pattern of a binding) - rust-lang/rust#153920 (improve `#[track_caller]` invalid ABI error) - rust-lang/rust#153946 (dissolve `tests/ui/cross`) - rust-lang/rust#153965 (Fix minor kasan bugs) - rust-lang/rust#153991 (Small report_cycle refactor)
…uwer Rollup of 14 pull requests Successful merges: - rust-lang/rust#153972 (stdarch subtree update) - rust-lang/rust#153801 (Add the option to run UI tests with the parallel frontend) - rust-lang/rust#153959 (Fix non-module `parent_module` in stripped cfg diagnostics) - rust-lang/rust#153967 (Tweak wording of failed predicate in inference error) - rust-lang/rust#152968 (Flip "region lattice" in RegionKind doc comment) - rust-lang/rust#153531 (Fix LegacyKeyValueFormat report from docker build: various) - rust-lang/rust#153622 (remove concept of soft-unstable features) - rust-lang/rust#153709 (Fix hypothetical ICE in `variances_of`) - rust-lang/rust#153884 (test `classify-runtime-const` for `f16`) - rust-lang/rust#153894 (Point at unit structs on foreign crates in type errors when they are the pattern of a binding) - rust-lang/rust#153920 (improve `#[track_caller]` invalid ABI error) - rust-lang/rust#153946 (dissolve `tests/ui/cross`) - rust-lang/rust#153965 (Fix minor kasan bugs) - rust-lang/rust#153991 (Small report_cycle refactor)
Consts and unit structs in patterns can be confusing if they are mistaken for new bindings. We already provide some context for unit structs and consts that come from the current crate, we now also point at those from foreign crates, and we properly skip cases where the pattern has type parameters which can't be confused with a new binding. So not suggest making a new binding when other suggestions are already emitted, as the likelihood of the other suggestions being what the user intended is higher. Make new binding suggestion verbose.
Fix #129792.