From 4e7ec07bb910aef258a848d5fcfa96b3bab7b3b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 26 Mar 2019 19:04:25 -0700 Subject: [PATCH 1/4] Account for short-hand field syntax when suggesting borrow --- src/librustc_typeck/check/demand.rs | 48 ++++++++++++++++++++++++----- src/test/ui/deref-suggestion.rs | 14 +++++++++ src/test/ui/deref-suggestion.stderr | 32 ++++++++++++++++--- 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index b1a249d821bec..5939f965269be 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -277,6 +277,24 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { return None; } + let parent_id = self.tcx.hir().get_parent_node_by_hir_id(expr.hir_id); + let mut is_struct_pat_shorthand_field = false; + if let Some(parent) = self.tcx.hir().find_by_hir_id(parent_id) { + // Account for fields + if let Node::Expr(hir::Expr { + node: hir::ExprKind::Struct(_, fields, ..), .. + }) = parent { + if let Ok(src) = cm.span_to_snippet(sp) { + for field in fields { + if field.ident.as_str() == src.as_str() && field.is_shorthand { + is_struct_pat_shorthand_field = true; + break; + } + } + } + } + }; + match (&expected.sty, &checked_ty.sty) { (&ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.sty, &check.sty) { (&ty::Str, &ty::Array(arr, _)) | @@ -341,14 +359,22 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(sugg) = self.can_use_as_ref(expr) { return Some(sugg); } - return Some(match mutability { - hir::Mutability::MutMutable => { - (sp, "consider mutably borrowing here", format!("&mut {}", - sugg_expr)) + return Some(match (mutability, is_struct_pat_shorthand_field) { + (hir::Mutability::MutMutable, false) => { + (sp, "consider mutably borrowing here", + format!("&mut {}", sugg_expr)) } - hir::Mutability::MutImmutable => { + (hir::Mutability::MutImmutable, false) => { (sp, "consider borrowing here", format!("&{}", sugg_expr)) } + (hir::Mutability::MutMutable, true) => { + (sp, "consider mutably borrowing here", + format!("{}: &mut {}", sugg_expr, sugg_expr)) + } + (hir::Mutability::MutImmutable, true) => { + (sp, "consider borrowing here", + format!("{}: &{}", sugg_expr, sugg_expr)) + } }); } } @@ -389,12 +415,18 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { checked, sp) { // do not suggest if the span comes from a macro (#52783) - if let (Ok(code), - true) = (cm.span_to_snippet(sp), sp == expr.span) { + if let (Ok(code), true) = ( + cm.span_to_snippet(sp), + sp == expr.span, + ) { return Some(( sp, "consider dereferencing the borrow", - format!("*{}", code), + if is_struct_pat_shorthand_field { + format!("{}: *{}", code, code) + } else { + format!("*{}", code) + }, )); } } diff --git a/src/test/ui/deref-suggestion.rs b/src/test/ui/deref-suggestion.rs index 83e54b64f47c9..f156766f52881 100644 --- a/src/test/ui/deref-suggestion.rs +++ b/src/test/ui/deref-suggestion.rs @@ -15,6 +15,14 @@ fn foo4(u: &u32) { //~^ ERROR mismatched types } +struct S<'a> { + u: &'a u32, +} + +struct R { + i: u32, +} + fn main() { let s = String::new(); let r_s = &s; @@ -27,4 +35,10 @@ fn main() { foo4(&0); assert_eq!(3i32, &3i32); //~^ ERROR mismatched types + let u = 3; + let s = S { u }; + //~^ ERROR mismatched types + let i = &4; + let r = R { i }; + //~^ ERROR mismatched types } diff --git a/src/test/ui/deref-suggestion.stderr b/src/test/ui/deref-suggestion.stderr index 8f061b3416e13..bd0ebfac5319d 100644 --- a/src/test/ui/deref-suggestion.stderr +++ b/src/test/ui/deref-suggestion.stderr @@ -23,7 +23,7 @@ LL | foo3(u); found type `&u32` error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:22:9 + --> $DIR/deref-suggestion.rs:30:9 | LL | foo(&"aaa".to_owned()); | ^^^^^^^^^^^^^^^^^ @@ -35,7 +35,7 @@ LL | foo(&"aaa".to_owned()); found type `&std::string::String` error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:24:9 + --> $DIR/deref-suggestion.rs:32:9 | LL | foo(&mut "aaa".to_owned()); | ^^^^^^^^^^^^^^^^^^^^^ @@ -59,7 +59,7 @@ LL | foo3(borrow!(0)); found type `&{integer}` error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:28:5 + --> $DIR/deref-suggestion.rs:36:5 | LL | assert_eq!(3i32, &3i32); | ^^^^^^^^^^^^^^^^^^^^^^^^ expected i32, found &i32 @@ -68,6 +68,30 @@ LL | assert_eq!(3i32, &3i32); found type `&i32` = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: aborting due to 6 previous errors +error[E0308]: mismatched types + --> $DIR/deref-suggestion.rs:39:17 + | +LL | let s = S { u }; + | ^ + | | + | expected &u32, found integer + | help: consider borrowing here: `u: &u` + | + = note: expected type `&u32` + found type `{integer}` + +error[E0308]: mismatched types + --> $DIR/deref-suggestion.rs:42:17 + | +LL | let r = R { i }; + | ^ + | | + | expected u32, found &{integer} + | help: consider dereferencing the borrow: `i: *i` + | + = note: expected type `u32` + found type `&{integer}` + +error: aborting due to 8 previous errors For more information about this error, try `rustc --explain E0308`. From a51ca0268d0b130cbe3ef4a2dd0024d6d136e3fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 26 Mar 2019 19:07:15 -0700 Subject: [PATCH 2/4] Expand test --- src/test/ui/deref-suggestion.rs | 4 ++++ src/test/ui/deref-suggestion.stderr | 28 ++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/test/ui/deref-suggestion.rs b/src/test/ui/deref-suggestion.rs index f156766f52881..580410aecf4f8 100644 --- a/src/test/ui/deref-suggestion.rs +++ b/src/test/ui/deref-suggestion.rs @@ -38,7 +38,11 @@ fn main() { let u = 3; let s = S { u }; //~^ ERROR mismatched types + let s = S { u: u }; + //~^ ERROR mismatched types let i = &4; let r = R { i }; //~^ ERROR mismatched types + let r = R { i: i }; + //~^ ERROR mismatched types } diff --git a/src/test/ui/deref-suggestion.stderr b/src/test/ui/deref-suggestion.stderr index bd0ebfac5319d..9c49f541c9309 100644 --- a/src/test/ui/deref-suggestion.stderr +++ b/src/test/ui/deref-suggestion.stderr @@ -81,7 +81,19 @@ LL | let s = S { u }; found type `{integer}` error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:42:17 + --> $DIR/deref-suggestion.rs:41:20 + | +LL | let s = S { u: u }; + | ^ + | | + | expected &u32, found integer + | help: consider borrowing here: `&u` + | + = note: expected type `&u32` + found type `{integer}` + +error[E0308]: mismatched types + --> $DIR/deref-suggestion.rs:44:17 | LL | let r = R { i }; | ^ @@ -92,6 +104,18 @@ LL | let r = R { i }; = note: expected type `u32` found type `&{integer}` -error: aborting due to 8 previous errors +error[E0308]: mismatched types + --> $DIR/deref-suggestion.rs:46:20 + | +LL | let r = R { i: i }; + | ^ + | | + | expected u32, found &{integer} + | help: consider dereferencing the borrow: `*i` + | + = note: expected type `u32` + found type `&{integer}` + +error: aborting due to 10 previous errors For more information about this error, try `rustc --explain E0308`. From 07857f74065c567a46549cb88d6a0aba3cef484c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 28 Mar 2019 04:47:09 -0700 Subject: [PATCH 3/4] review comments --- src/librustc_typeck/check/demand.rs | 83 +++++++++++++++-------------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 5939f965269be..7886af0c97393 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -249,6 +249,26 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { None } + fn is_hir_id_from_struct_pattern_shorthand_field(&self, hir_id: hir::HirId) -> bool { + let parent_id = self.tcx.hir().get_parent_node_by_hir_id(expr.hir_id); + let mut is_struct_pat_shorthand_field = false; + if let Some(parent) = self.tcx.hir().find_by_hir_id(parent_id) { + // Account for fields + if let Node::Expr(hir::Expr { + node: hir::ExprKind::Struct(_, fields, ..), .. + }) = parent { + if let Ok(src) = cm.span_to_snippet(sp) { + for field in fields { + if field.ident.as_str() == src.as_str() && field.is_shorthand { + is_struct_pat_shorthand_field = true; + break; + } + } + } + } + }; + } + /// This function is used to determine potential "simple" improvements or users' errors and /// provide them useful help. For example: /// @@ -277,23 +297,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { return None; } - let parent_id = self.tcx.hir().get_parent_node_by_hir_id(expr.hir_id); - let mut is_struct_pat_shorthand_field = false; - if let Some(parent) = self.tcx.hir().find_by_hir_id(parent_id) { - // Account for fields - if let Node::Expr(hir::Expr { - node: hir::ExprKind::Struct(_, fields, ..), .. - }) = parent { - if let Ok(src) = cm.span_to_snippet(sp) { - for field in fields { - if field.ident.as_str() == src.as_str() && field.is_shorthand { - is_struct_pat_shorthand_field = true; - break; - } - } - } - } - }; + let mut is_struct_pat_shorthand_field = + self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id); match (&expected.sty, &checked_ty.sty) { (&ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.sty, &check.sty) { @@ -333,12 +338,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // bar(&x); // error, expected &mut // ``` let ref_ty = match mutability { - hir::Mutability::MutMutable => self.tcx.mk_mut_ref( - self.tcx.mk_region(ty::ReStatic), - checked_ty), - hir::Mutability::MutImmutable => self.tcx.mk_imm_ref( - self.tcx.mk_region(ty::ReStatic), - checked_ty), + hir::Mutability::MutMutable => { + self.tcx.mk_mut_ref(self.tcx.mk_region(ty::ReStatic), checked_ty) + } + hir::Mutability::MutImmutable => { + self.tcx.mk_imm_ref(self.tcx.mk_region(ty::ReStatic), checked_ty) + } }; if self.can_coerce(ref_ty, expected) { if let Ok(src) = cm.span_to_snippet(sp) { @@ -359,22 +364,22 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(sugg) = self.can_use_as_ref(expr) { return Some(sugg); } - return Some(match (mutability, is_struct_pat_shorthand_field) { - (hir::Mutability::MutMutable, false) => { - (sp, "consider mutably borrowing here", - format!("&mut {}", sugg_expr)) - } - (hir::Mutability::MutImmutable, false) => { - (sp, "consider borrowing here", format!("&{}", sugg_expr)) - } - (hir::Mutability::MutMutable, true) => { - (sp, "consider mutably borrowing here", - format!("{}: &mut {}", sugg_expr, sugg_expr)) - } - (hir::Mutability::MutImmutable, true) => { - (sp, "consider borrowing here", - format!("{}: &{}", sugg_expr, sugg_expr)) - } + let field_name = if is_struct_pat_shorthand_field { + format!("{}: ", sugg_expr) + } else { + String::new() + }; + return Some(match mutability { + hir::Mutability::MutMutable => ( + sp, + "consider mutably borrowing here", + format!("{}&mut {}", field_name, sugg_expr), + ), + hir::Mutability::MutImmutable => ( + sp, + "consider borrowing here", + format!("{}&{}", field_name, sugg_expr), + ), }); } } From ddfa47f4b4926d411d52afa0e29dead708c2a9d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 29 Mar 2019 06:46:39 -0700 Subject: [PATCH 4/4] Fix incorrect code --- src/librustc_typeck/check/demand.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 7886af0c97393..cca6da4f82c32 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -249,9 +249,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { None } - fn is_hir_id_from_struct_pattern_shorthand_field(&self, hir_id: hir::HirId) -> bool { - let parent_id = self.tcx.hir().get_parent_node_by_hir_id(expr.hir_id); - let mut is_struct_pat_shorthand_field = false; + fn is_hir_id_from_struct_pattern_shorthand_field(&self, hir_id: hir::HirId, sp: Span) -> bool { + let cm = self.sess().source_map(); + let parent_id = self.tcx.hir().get_parent_node_by_hir_id(hir_id); if let Some(parent) = self.tcx.hir().find_by_hir_id(parent_id) { // Account for fields if let Node::Expr(hir::Expr { @@ -260,13 +260,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Ok(src) = cm.span_to_snippet(sp) { for field in fields { if field.ident.as_str() == src.as_str() && field.is_shorthand { - is_struct_pat_shorthand_field = true; - break; + return true; } } } } - }; + } + false } /// This function is used to determine potential "simple" improvements or users' errors and @@ -297,8 +297,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { return None; } - let mut is_struct_pat_shorthand_field = - self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id); + let is_struct_pat_shorthand_field = self.is_hir_id_from_struct_pattern_shorthand_field( + expr.hir_id, + sp, + ); match (&expected.sty, &checked_ty.sty) { (&ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.sty, &check.sty) {