From 0e7e9382fa1a6e50c976cd74ca420d7cb10591fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Apr 2019 18:12:57 -0700 Subject: [PATCH 1/7] Do not suggest incorrect syntax on pattern borrow error --- src/librustc_typeck/check/_match.rs | 25 ++++++++++++++----- src/test/ui/destructure-trait-ref.stderr | 2 -- .../ui/mismatched_types/issue-38371.stderr | 1 - 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index f3cabced34dfc..715b82183a761 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -378,12 +378,25 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // `fn foo(foo: &u32)` if let Some(mut err) = err { if let PatKind::Binding(..) = inner.node { - if let Ok(snippet) = tcx.sess.source_map() - .span_to_snippet(pat.span) - { - err.help(&format!("did you mean `{}: &{}`?", - &snippet[1..], - expected)); + let parent_id = tcx.hir().get_parent_node_by_hir_id(pat.hir_id); + let parent = tcx.hir().get_by_hir_id(parent_id); + match parent { + hir::Node::Item(_) | + hir::Node::ForeignItem(_) | + hir::Node::TraitItem(_) | + hir::Node::ImplItem(_) => { // this pat is an argument + if let Ok(snippet) = tcx.sess.source_map() + .span_to_snippet(pat.span) + { // FIXME: turn into structured suggestion, will need + // a span that also includes the the type. + err.help(&format!( + "did you mean `{}: &{}`?", + &snippet[1..], + expected, + )); + } + } + _ => {} // don't provide the suggestion from above #55175 } } err.emit(); diff --git a/src/test/ui/destructure-trait-ref.stderr b/src/test/ui/destructure-trait-ref.stderr index 34dd213e2b390..7f389299afba8 100644 --- a/src/test/ui/destructure-trait-ref.stderr +++ b/src/test/ui/destructure-trait-ref.stderr @@ -24,7 +24,6 @@ LL | let &&x = &1isize as &T; | = note: expected type `dyn T` found type `&_` - = help: did you mean `x: &dyn T`? error[E0308]: mismatched types --> $DIR/destructure-trait-ref.rs:36:11 @@ -34,7 +33,6 @@ LL | let &&&x = &(&1isize as &T); | = note: expected type `dyn T` found type `&_` - = help: did you mean `x: &dyn T`? error[E0308]: mismatched types --> $DIR/destructure-trait-ref.rs:41:13 diff --git a/src/test/ui/mismatched_types/issue-38371.stderr b/src/test/ui/mismatched_types/issue-38371.stderr index 236f742db3f71..30da48ba4a8ad 100644 --- a/src/test/ui/mismatched_types/issue-38371.stderr +++ b/src/test/ui/mismatched_types/issue-38371.stderr @@ -16,7 +16,6 @@ LL | fn agh(&&bar: &u32) { | = note: expected type `u32` found type `&_` - = help: did you mean `bar: &u32`? error[E0308]: mismatched types --> $DIR/issue-38371.rs:21:8 From 742b48dc39768fbafd5a072b4ee1dbcd207d12f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Apr 2019 19:02:54 -0700 Subject: [PATCH 2/7] Be more specific in the suggestion filtering --- src/librustc_typeck/check/_match.rs | 52 ++++++++++++++++--- src/test/ui/destructure-trait-ref.stderr | 10 +++- .../ui/mismatched_types/issue-38371.stderr | 5 +- 3 files changed, 56 insertions(+), 11 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 715b82183a761..53d1ac7b02496 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -380,23 +380,59 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let PatKind::Binding(..) = inner.node { let parent_id = tcx.hir().get_parent_node_by_hir_id(pat.hir_id); let parent = tcx.hir().get_by_hir_id(parent_id); + debug!("inner {:?} pat {:?} parent {:?}", inner, pat, parent); match parent { - hir::Node::Item(_) | - hir::Node::ForeignItem(_) | - hir::Node::TraitItem(_) | - hir::Node::ImplItem(_) => { // this pat is an argument + hir::Node::Item(hir::Item { + node: hir::ItemKind::Fn(..), .. + }) | + hir::Node::ForeignItem(hir::ForeignItem { + node: hir::ForeignItemKind::Fn(..), .. + }) | + hir::Node::TraitItem(hir::TraitItem { + node: hir::TraitItemKind::Method(..), .. + }) | + hir::Node::ImplItem(hir::ImplItem { + node: hir::ImplItemKind::Method(..), .. + }) => { // this pat is likely an argument if let Ok(snippet) = tcx.sess.source_map() - .span_to_snippet(pat.span) + .span_to_snippet(inner.span) { // FIXME: turn into structured suggestion, will need - // a span that also includes the the type. + // a span that also includes the the arg's type. err.help(&format!( "did you mean `{}: &{}`?", - &snippet[1..], + snippet, expected, )); } } - _ => {} // don't provide the suggestion from above #55175 + hir::Node::Expr(hir::Expr { + node: hir::ExprKind::Match(..), .. + }) => { // rely on match ergonomics + if let Ok(snippet) = tcx.sess.source_map() + .span_to_snippet(inner.span) + { + err.span_suggestion( + pat.span, + "you can rely on match ergonomics and remove \ + the explicit borrow", + snippet, + Applicability::MaybeIncorrect, + ); + } + } + hir::Node::Pat(_) => { // nested `&&pat` + if let Ok(snippet) = tcx.sess.source_map() + .span_to_snippet(inner.span) + { + err.span_suggestion( + pat.span, + "you can probaly remove the explicit borrow", + snippet, + Applicability::MaybeIncorrect, + ); + } + } + _ => {} // don't provide suggestions in other cases #55175 } } err.emit(); diff --git a/src/test/ui/destructure-trait-ref.stderr b/src/test/ui/destructure-trait-ref.stderr index 7f389299afba8..47ecadfd01afb 100644 --- a/src/test/ui/destructure-trait-ref.stderr +++ b/src/test/ui/destructure-trait-ref.stderr @@ -20,7 +20,10 @@ error[E0308]: mismatched types --> $DIR/destructure-trait-ref.rs:31:10 | LL | let &&x = &1isize as &T; - | ^^ expected trait T, found reference + | ^^ + | | + | expected trait T, found reference + | help: you can probaly remove the explicit borrow: `x` | = note: expected type `dyn T` found type `&_` @@ -29,7 +32,10 @@ error[E0308]: mismatched types --> $DIR/destructure-trait-ref.rs:36:11 | LL | let &&&x = &(&1isize as &T); - | ^^ expected trait T, found reference + | ^^ + | | + | expected trait T, found reference + | help: you can probaly remove the explicit borrow: `x` | = note: expected type `dyn T` found type `&_` diff --git a/src/test/ui/mismatched_types/issue-38371.stderr b/src/test/ui/mismatched_types/issue-38371.stderr index 30da48ba4a8ad..833b8998c338a 100644 --- a/src/test/ui/mismatched_types/issue-38371.stderr +++ b/src/test/ui/mismatched_types/issue-38371.stderr @@ -12,7 +12,10 @@ error[E0308]: mismatched types --> $DIR/issue-38371.rs:18:9 | LL | fn agh(&&bar: &u32) { - | ^^^^ expected u32, found reference + | ^^^^ + | | + | expected u32, found reference + | help: you can probaly remove the explicit borrow: `bar` | = note: expected type `u32` found type `&_` From ff6867338792042b13d401d7c2c3f0aee8d04e62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Apr 2019 19:16:35 -0700 Subject: [PATCH 3/7] add tests --- src/test/ui/suggestions/match-ergonomics.rs | 40 ++++++++++++++++++ .../ui/suggestions/match-ergonomics.stderr | 41 +++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 src/test/ui/suggestions/match-ergonomics.rs create mode 100644 src/test/ui/suggestions/match-ergonomics.stderr diff --git a/src/test/ui/suggestions/match-ergonomics.rs b/src/test/ui/suggestions/match-ergonomics.rs new file mode 100644 index 0000000000000..d75f8363cd221 --- /dev/null +++ b/src/test/ui/suggestions/match-ergonomics.rs @@ -0,0 +1,40 @@ +fn main() { + let x = vec![1i32]; + match &x[..] { + [&v] => {}, //~ ERROR mismatched types + _ => {}, + } + match x { + [&v] => {}, //~ ERROR expected an array or slice + _ => {}, + } + match &x[..] { + [v] => {}, + _ => {}, + } + match &x[..] { + &[v] => {}, + _ => {}, + } + match x { + [v] => {}, //~ ERROR expected an array or slice + _ => {}, + } + let y = 1i32; + match &y { + &v => {}, + _ => {}, + } + match y { + &v => {}, //~ ERROR mismatched types + _ => {}, + } + match &y { + v => {}, + _ => {}, + } + match y { + v => {}, + _ => {}, + } +} diff --git a/src/test/ui/suggestions/match-ergonomics.stderr b/src/test/ui/suggestions/match-ergonomics.stderr new file mode 100644 index 0000000000000..4239089b7cec7 --- /dev/null +++ b/src/test/ui/suggestions/match-ergonomics.stderr @@ -0,0 +1,41 @@ +error[E0308]: mismatched types + --> $DIR/match-ergonomics.rs:4:10 + | +LL | [&v] => {}, + | ^^ + | | + | expected i32, found reference + | help: you can probaly remove the explicit borrow: `v` + | + = note: expected type `i32` + found type `&_` + +error[E0529]: expected an array or slice, found `std::vec::Vec` + --> $DIR/match-ergonomics.rs:8:9 + | +LL | [&v] => {}, + | ^^^^ pattern cannot match with input type `std::vec::Vec` + +error[E0529]: expected an array or slice, found `std::vec::Vec` + --> $DIR/match-ergonomics.rs:20:9 + | +LL | [v] => {}, + | ^^^ pattern cannot match with input type `std::vec::Vec` + +error[E0308]: mismatched types + --> $DIR/match-ergonomics.rs:29:9 + | +LL | &v => {}, + | ^^ expected i32, found reference + | + = note: expected type `i32` + found type `&_` +help: you can rely on match ergonomics and remove the explicit borrow + | +LL | v => {}, + | ^ + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0308, E0529. +For more information about an error, try `rustc --explain E0308`. From 693eea5784fc58e9d9a8cade7a5b2afa2f3dbd3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Apr 2019 19:56:50 -0700 Subject: [PATCH 4/7] review comments --- src/librustc_typeck/check/_match.rs | 116 +++++++++--------- src/test/ui/destructure-trait-ref.stderr | 4 +- .../ui/mismatched_types/issue-38371.stderr | 2 +- .../ui/suggestions/match-ergonomics.stderr | 2 +- 4 files changed, 60 insertions(+), 64 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 53d1ac7b02496..124afc5491e89 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -1,8 +1,8 @@ use crate::check::{FnCtxt, Expectation, Diverges, Needs}; use crate::check::coercion::CoerceMany; use crate::util::nodemap::FxHashMap; -use errors::Applicability; -use rustc::hir::{self, PatKind}; +use errors::{Applicability, DiagnosticBuilder}; +use rustc::hir::{self, PatKind, Pat}; use rustc::hir::def::{Def, CtorKind}; use rustc::hir::pat_util::EnumerateAndAdjustIterator; use rustc::infer; @@ -377,64 +377,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // Look for a case like `fn foo(&foo: u32)` and suggest // `fn foo(foo: &u32)` if let Some(mut err) = err { - if let PatKind::Binding(..) = inner.node { - let parent_id = tcx.hir().get_parent_node_by_hir_id(pat.hir_id); - let parent = tcx.hir().get_by_hir_id(parent_id); - debug!("inner {:?} pat {:?} parent {:?}", inner, pat, parent); - match parent { - hir::Node::Item(hir::Item { - node: hir::ItemKind::Fn(..), .. - }) | - hir::Node::ForeignItem(hir::ForeignItem { - node: hir::ForeignItemKind::Fn(..), .. - }) | - hir::Node::TraitItem(hir::TraitItem { - node: hir::TraitItemKind::Method(..), .. - }) | - hir::Node::ImplItem(hir::ImplItem { - node: hir::ImplItemKind::Method(..), .. - }) => { // this pat is likely an argument - if let Ok(snippet) = tcx.sess.source_map() - .span_to_snippet(inner.span) - { // FIXME: turn into structured suggestion, will need - // a span that also includes the the arg's type. - err.help(&format!( - "did you mean `{}: &{}`?", - snippet, - expected, - )); - } - } - hir::Node::Expr(hir::Expr { - node: hir::ExprKind::Match(..), .. - }) => { // rely on match ergonomics - if let Ok(snippet) = tcx.sess.source_map() - .span_to_snippet(inner.span) - { - err.span_suggestion( - pat.span, - "you can rely on match ergonomics and remove \ - the explicit borrow", - snippet, - Applicability::MaybeIncorrect, - ); - } - } - hir::Node::Pat(_) => { // nested `&&pat` - if let Ok(snippet) = tcx.sess.source_map() - .span_to_snippet(inner.span) - { - err.span_suggestion( - pat.span, - "you can probaly remove the explicit borrow", - snippet, - Applicability::MaybeIncorrect, - ); - } - } - _ => {} // don't provide suggestions in other cases #55175 - } - } + self.borrow_pat_suggestion(&mut err, &pat, &inner, &expected); err.emit(); } (rptr_ty, inner_ty) @@ -566,6 +509,59 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // subtyping. } + fn borrow_pat_suggestion( + &self, + err: &mut DiagnosticBuilder<'_>, + pat: &Pat, + inner: &Pat, + expected: &Ty<'tcx>, + ) { + let tcx = self.tcx; + if let PatKind::Binding(..) = inner.node { + let parent_id = tcx.hir().get_parent_node_by_hir_id(pat.hir_id); + let parent = tcx.hir().get_by_hir_id(parent_id); + debug!("inner {:?} pat {:?} parent {:?}", inner, pat, parent); + match parent { + hir::Node::Item(hir::Item { node: hir::ItemKind::Fn(..), .. }) | + hir::Node::ForeignItem(hir::ForeignItem { + node: hir::ForeignItemKind::Fn(..), .. + }) | + hir::Node::TraitItem(hir::TraitItem { node: hir::TraitItemKind::Method(..), .. }) | + hir::Node::ImplItem(hir::ImplItem { node: hir::ImplItemKind::Method(..), .. }) => { + // this pat is likely an argument + if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(inner.span) { + // FIXME: turn into structured suggestion, will need a span that also + // includes the the arg's type. + err.help(&format!("did you mean `{}: &{}`?", snippet, expected)); + } + } + hir::Node::Expr(hir::Expr { node: hir::ExprKind::Match(..), .. }) => { + // rely on match ergonomics + if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(inner.span) { + err.span_suggestion( + pat.span, + "you can rely on match ergonomics and remove the explicit borrow", + snippet, + Applicability::MaybeIncorrect, + ); + } + } + hir::Node::Pat(_) => { + // nested `&&pat` + if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(inner.span) { + err.span_suggestion( + pat.span, + "you can probably remove the explicit borrow", + snippet, + Applicability::MaybeIncorrect, + ); + } + } + _ => {} // don't provide suggestions in other cases #55175 + } + } + } + pub fn check_dereferencable(&self, span: Span, expected: Ty<'tcx>, inner: &hir::Pat) -> bool { if let PatKind::Binding(..) = inner.node { if let Some(mt) = self.shallow_resolve(expected).builtin_deref(true) { diff --git a/src/test/ui/destructure-trait-ref.stderr b/src/test/ui/destructure-trait-ref.stderr index 47ecadfd01afb..bc3013b78b38c 100644 --- a/src/test/ui/destructure-trait-ref.stderr +++ b/src/test/ui/destructure-trait-ref.stderr @@ -23,7 +23,7 @@ LL | let &&x = &1isize as &T; | ^^ | | | expected trait T, found reference - | help: you can probaly remove the explicit borrow: `x` + | help: you can probably remove the explicit borrow: `x` | = note: expected type `dyn T` found type `&_` @@ -35,7 +35,7 @@ LL | let &&&x = &(&1isize as &T); | ^^ | | | expected trait T, found reference - | help: you can probaly remove the explicit borrow: `x` + | help: you can probably remove the explicit borrow: `x` | = note: expected type `dyn T` found type `&_` diff --git a/src/test/ui/mismatched_types/issue-38371.stderr b/src/test/ui/mismatched_types/issue-38371.stderr index 833b8998c338a..a9347926bda0a 100644 --- a/src/test/ui/mismatched_types/issue-38371.stderr +++ b/src/test/ui/mismatched_types/issue-38371.stderr @@ -15,7 +15,7 @@ LL | fn agh(&&bar: &u32) { | ^^^^ | | | expected u32, found reference - | help: you can probaly remove the explicit borrow: `bar` + | help: you can probably remove the explicit borrow: `bar` | = note: expected type `u32` found type `&_` diff --git a/src/test/ui/suggestions/match-ergonomics.stderr b/src/test/ui/suggestions/match-ergonomics.stderr index 4239089b7cec7..a064e2485ffe8 100644 --- a/src/test/ui/suggestions/match-ergonomics.stderr +++ b/src/test/ui/suggestions/match-ergonomics.stderr @@ -5,7 +5,7 @@ LL | [&v] => {}, | ^^ | | | expected i32, found reference - | help: you can probaly remove the explicit borrow: `v` + | help: you can probably remove the explicit borrow: `v` | = note: expected type `i32` found type `&_` From 14ca95066579f4ec3761fa171e9c9c6d7bd80ccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Apr 2019 20:50:04 -0700 Subject: [PATCH 5/7] Fix style --- src/librustc_typeck/check/_match.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 124afc5491e89..88168a6950be4 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -514,7 +514,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { err: &mut DiagnosticBuilder<'_>, pat: &Pat, inner: &Pat, - expected: &Ty<'tcx>, + expected: Ty<'tcx>, ) { let tcx = self.tcx; if let PatKind::Binding(..) = inner.node { From 6068478d56a05ab1aa4d9ad87046e1d5d47afd7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Apr 2019 20:58:29 -0700 Subject: [PATCH 6/7] Add if let test --- src/test/ui/suggestions/match-ergonomics.rs | 1 + src/test/ui/suggestions/match-ergonomics.stderr | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/test/ui/suggestions/match-ergonomics.rs b/src/test/ui/suggestions/match-ergonomics.rs index d75f8363cd221..c4fc01469bf65 100644 --- a/src/test/ui/suggestions/match-ergonomics.rs +++ b/src/test/ui/suggestions/match-ergonomics.rs @@ -37,4 +37,5 @@ fn main() { v => {}, _ => {}, } + if let [&v] = &x[..] {} //~ ERROR mismatched types } diff --git a/src/test/ui/suggestions/match-ergonomics.stderr b/src/test/ui/suggestions/match-ergonomics.stderr index a064e2485ffe8..9915eeb34fac1 100644 --- a/src/test/ui/suggestions/match-ergonomics.stderr +++ b/src/test/ui/suggestions/match-ergonomics.stderr @@ -35,7 +35,19 @@ help: you can rely on match ergonomics and remove the explicit borrow LL | v => {}, | ^ -error: aborting due to 4 previous errors +error[E0308]: mismatched types + --> $DIR/match-ergonomics.rs:40:13 + | +LL | if let [&v] = &x[..] {} + | ^^ + | | + | expected i32, found reference + | help: you can probably remove the explicit borrow: `v` + | + = note: expected type `i32` + found type `&_` + +error: aborting due to 5 previous errors Some errors have detailed explanations: E0308, E0529. For more information about an error, try `rustc --explain E0308`. From ed08c6a985d0f69d2eac33e09440112c880c9fd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 30 Apr 2019 10:59:30 -0700 Subject: [PATCH 7/7] review comments: change wording --- src/librustc_typeck/check/_match.rs | 14 ++------------ src/test/ui/suggestions/match-ergonomics.stderr | 9 ++++----- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 88168a6950be4..f5aa2e6ea5cea 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -535,19 +535,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { err.help(&format!("did you mean `{}: &{}`?", snippet, expected)); } } - hir::Node::Expr(hir::Expr { node: hir::ExprKind::Match(..), .. }) => { - // rely on match ergonomics - if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(inner.span) { - err.span_suggestion( - pat.span, - "you can rely on match ergonomics and remove the explicit borrow", - snippet, - Applicability::MaybeIncorrect, - ); - } - } + hir::Node::Expr(hir::Expr { node: hir::ExprKind::Match(..), .. }) | hir::Node::Pat(_) => { - // nested `&&pat` + // rely on match ergonomics or it might be nested `&&pat` if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(inner.span) { err.span_suggestion( pat.span, diff --git a/src/test/ui/suggestions/match-ergonomics.stderr b/src/test/ui/suggestions/match-ergonomics.stderr index 9915eeb34fac1..b7497be6ceb36 100644 --- a/src/test/ui/suggestions/match-ergonomics.stderr +++ b/src/test/ui/suggestions/match-ergonomics.stderr @@ -26,14 +26,13 @@ error[E0308]: mismatched types --> $DIR/match-ergonomics.rs:29:9 | LL | &v => {}, - | ^^ expected i32, found reference + | ^^ + | | + | expected i32, found reference + | help: you can probably remove the explicit borrow: `v` | = note: expected type `i32` found type `&_` -help: you can rely on match ergonomics and remove the explicit borrow - | -LL | v => {}, - | ^ error[E0308]: mismatched types --> $DIR/match-ergonomics.rs:40:13