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.

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

Suggest adding/removing ref for binding patterns #99835

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 49 additions & 4 deletions compiler/rustc_typeck/src/check/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// If there are multiple arms, make sure they all agree on
// what the type of the binding `x` ought to be.
if var_id != pat.hir_id {
self.check_binding_alt_eq_ty(pat.span, var_id, local_ty, ti);
self.check_binding_alt_eq_ty(ba, pat.span, var_id, local_ty, ti);
}

if let Some(p) = sub {
Expand All @@ -627,7 +627,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
local_ty
}

fn check_binding_alt_eq_ty(&self, span: Span, var_id: HirId, ty: Ty<'tcx>, ti: TopInfo<'tcx>) {
fn check_binding_alt_eq_ty(
&self,
ba: hir::BindingAnnotation,
span: Span,
var_id: HirId,
ty: Ty<'tcx>,
ti: TopInfo<'tcx>,
) {
let var_ty = self.local_ty(span, var_id).decl_ty;
if let Some(mut err) = self.demand_eqtype_pat_diag(span, var_ty, ty, ti) {
let hir = self.tcx.hir();
Expand All @@ -645,12 +652,50 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
});
let pre = if in_match { "in the same arm, " } else { "" };
err.note(&format!("{}a binding must have the same type in all alternatives", pre));
// FIXME: check if `var_ty` and `ty` can be made the same type by adding or removing
// `ref` or `&` to the pattern.
self.suggest_adding_missing_ref_or_removing_ref(
&mut err,
span,
var_ty,
self.resolve_vars_with_obligations(ty),
ba,
);
err.emit();
}
}

fn suggest_adding_missing_ref_or_removing_ref(
&self,
err: &mut Diagnostic,
span: Span,
expected: Ty<'tcx>,
actual: Ty<'tcx>,
ba: hir::BindingAnnotation,
) {
match (expected.kind(), actual.kind(), ba) {
(ty::Ref(_, inner_ty, _), _, hir::BindingAnnotation::Unannotated)
if self.can_eq(self.param_env, *inner_ty, actual).is_ok() =>
{
err.span_suggestion_verbose(
span.shrink_to_lo(),
"consider adding `ref`",
"ref ",
Applicability::MaybeIncorrect,
);
}
(_, ty::Ref(_, inner_ty, _), hir::BindingAnnotation::Ref)
if self.can_eq(self.param_env, expected, *inner_ty).is_ok() =>
{
err.span_suggestion_verbose(
span.with_hi(span.lo() + BytePos(4)),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should avoid this kind of span building when possible.

"consider removing `ref`",
"",
Applicability::MaybeIncorrect,
);
}
_ => (),
}
}

// Precondition: pat is a Ref(_) pattern
fn borrow_pat_suggestion(&self, err: &mut Diagnostic, pat: &Pat<'_>) {
let tcx = self.tcx;
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/mismatched_types/E0409.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ LL | (0, ref y) | (y, 0) => {}
| first introduced with type `&{integer}` here
|
= note: in the same arm, a binding must have the same type in all alternatives
help: consider adding `ref`
|
LL | (0, ref y) | (ref y, 0) => {}
| +++

error: aborting due to 2 previous errors

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// run-rustfix
#![allow(dead_code, unused_variables)]

fn main() {
enum Blah {
A(isize, isize, usize),
B(isize, usize),
}

match Blah::A(1, 1, 2) {
Blah::A(_, x, ref y) | Blah::B(x, ref y) => {}
//~^ ERROR mismatched types
//~| ERROR variable `y` is bound inconsistently across alternatives separated by `|`
}

match Blah::A(1, 1, 2) {
Blah::A(_, x, y) | Blah::B(x, y) => {}
//~^ ERROR mismatched types
//~| variable `y` is bound inconsistently across alternatives separated by `|`
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// run-rustfix
#![allow(dead_code, unused_variables)]

fn main() {
enum Blah {
A(isize, isize, usize),
B(isize, usize),
}

match Blah::A(1, 1, 2) {
Blah::A(_, x, ref y) | Blah::B(x, y) => {}
//~^ ERROR mismatched types
//~| ERROR variable `y` is bound inconsistently across alternatives separated by `|`
}

match Blah::A(1, 1, 2) {
Blah::A(_, x, y) | Blah::B(x, ref y) => {}
//~^ ERROR mismatched types
//~| variable `y` is bound inconsistently across alternatives separated by `|`
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
error[E0409]: variable `y` is bound inconsistently across alternatives separated by `|`
--> $DIR/suggest-adding-or-removing-ref-for-binding-pattern.rs:11:43
|
LL | Blah::A(_, x, ref y) | Blah::B(x, y) => {}
| - first binding ^ bound in different ways

error[E0409]: variable `y` is bound inconsistently across alternatives separated by `|`
--> $DIR/suggest-adding-or-removing-ref-for-binding-pattern.rs:17:43
|
LL | Blah::A(_, x, y) | Blah::B(x, ref y) => {}
| - first binding ^ bound in different ways

error[E0308]: mismatched types
--> $DIR/suggest-adding-or-removing-ref-for-binding-pattern.rs:11:43
|
LL | match Blah::A(1, 1, 2) {
| ---------------- this expression has type `Blah`
LL | Blah::A(_, x, ref y) | Blah::B(x, y) => {}
| ----- ^ expected `&usize`, found `usize`
| |
| first introduced with type `&usize` here
|
= note: in the same arm, a binding must have the same type in all alternatives
help: consider adding `ref`
|
LL | Blah::A(_, x, ref y) | Blah::B(x, ref y) => {}
| +++

error[E0308]: mismatched types
--> $DIR/suggest-adding-or-removing-ref-for-binding-pattern.rs:17:39
|
LL | match Blah::A(1, 1, 2) {
| ---------------- this expression has type `Blah`
LL | Blah::A(_, x, y) | Blah::B(x, ref y) => {}
| - ^^^^^ expected `usize`, found `&usize`
| |
| first introduced with type `usize` here
|
= note: in the same arm, a binding must have the same type in all alternatives
help: consider removing `ref`
|
LL - Blah::A(_, x, y) | Blah::B(x, ref y) => {}
LL + Blah::A(_, x, y) | Blah::B(x, y) => {}
|

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0308, E0409.
For more information about an error, try `rustc --explain E0308`.
8 changes: 8 additions & 0 deletions src/test/ui/resolve/resolve-inconsistent-binding-mode.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ LL | Opts::A(ref i) | Opts::B(i) => {}
| first introduced with type `&isize` here
|
= note: in the same arm, a binding must have the same type in all alternatives
help: consider adding `ref`
|
LL | Opts::A(ref i) | Opts::B(ref i) => {}
| +++

error[E0308]: mismatched types
--> $DIR/resolve-inconsistent-binding-mode.rs:18:34
Expand All @@ -43,6 +47,10 @@ LL | Opts::A(ref i) | Opts::B(i) => {}
| first introduced with type `&isize` here
|
= note: in the same arm, a binding must have the same type in all alternatives
help: consider adding `ref`
|
LL | Opts::A(ref i) | Opts::B(ref i) => {}
| +++

error[E0308]: mismatched types
--> $DIR/resolve-inconsistent-binding-mode.rs:27:38
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/resolve/resolve-inconsistent-names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ fn main() {
//~| ERROR mismatched types
//~| ERROR variable `c` is not bound in all patterns
//~| HELP if you meant to match on unit variant `E::A`, use the full path in the pattern
//~| HELP consider removing `ref`
}

let z = (10, 20);
Expand Down
9 changes: 7 additions & 2 deletions src/test/ui/resolve/resolve-inconsistent-names.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ LL | (A, B) | (ref B, c) | (c, A) => ()
| first binding

error[E0408]: variable `CONST1` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:30:23
--> $DIR/resolve-inconsistent-names.rs:31:23
|
LL | (CONST1, _) | (_, Const2) => ()
| ------ ^^^^^^^^^^^ pattern doesn't bind `CONST1`
Expand All @@ -69,7 +69,7 @@ LL | const CONST1: usize = 10;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ not accessible

error[E0408]: variable `Const2` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:30:9
--> $DIR/resolve-inconsistent-names.rs:31:9
|
LL | (CONST1, _) | (_, Const2) => ()
| ^^^^^^^^^^^ ------ variable not in all patterns
Expand All @@ -92,6 +92,11 @@ LL | (A, B) | (ref B, c) | (c, A) => ()
| first introduced with type `E` here
|
= note: in the same arm, a binding must have the same type in all alternatives
help: consider removing `ref`
|
LL - (A, B) | (ref B, c) | (c, A) => ()
LL + (A, B) | (B, c) | (c, A) => ()
|

error: aborting due to 9 previous errors

Expand Down