From c9a2616e44c4bced021f3599fe14c2177f27cadd Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 21 Apr 2019 18:49:02 +0100 Subject: [PATCH 1/3] Add existing behaviour test for deref suggestions. This commit adds a test that demonstrates the current behaviour where suggestions to add a dereference aren't given for non-references. --- src/test/ui/suggestions/issue-59819.rs | 20 ++++++++++++++++++ src/test/ui/suggestions/issue-59819.stderr | 24 ++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 src/test/ui/suggestions/issue-59819.rs create mode 100644 src/test/ui/suggestions/issue-59819.stderr diff --git a/src/test/ui/suggestions/issue-59819.rs b/src/test/ui/suggestions/issue-59819.rs new file mode 100644 index 0000000000000..b3c4df40161b0 --- /dev/null +++ b/src/test/ui/suggestions/issue-59819.rs @@ -0,0 +1,20 @@ +#![allow(warnings)] + +// Test that suggestion to add `*` characters applies to implementations of `Deref` as well as +// references. + +struct Foo(i32); + +impl std::ops::Deref for Foo { + type Target = i32; + fn deref(&self) -> &i32 { + &self.0 + } +} + +fn main() { + let x = Foo(42); + let y: i32 = x; //~ ERROR mismatched types + let a = &42; + let b: i32 = a; //~ ERROR mismatched types +} diff --git a/src/test/ui/suggestions/issue-59819.stderr b/src/test/ui/suggestions/issue-59819.stderr new file mode 100644 index 0000000000000..9c4d613466382 --- /dev/null +++ b/src/test/ui/suggestions/issue-59819.stderr @@ -0,0 +1,24 @@ +error[E0308]: mismatched types + --> $DIR/issue-59819.rs:17:18 + | +LL | let y: i32 = x; + | ^ expected i32, found struct `Foo` + | + = note: expected type `i32` + found type `Foo` + +error[E0308]: mismatched types + --> $DIR/issue-59819.rs:19:18 + | +LL | let b: i32 = a; + | ^ + | | + | expected i32, found &{integer} + | help: consider dereferencing the borrow: `*a` + | + = note: expected type `i32` + found type `&{integer}` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From fd95ba357465e39386924e40e05efef715a4ad46 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 21 Apr 2019 20:00:32 +0100 Subject: [PATCH 2/3] Suggest dereferencing when `Deref` is implemented. This commit suggests dereferencing a type when it implements `Deref` with the correct `Output` associated type. --- src/librustc_typeck/check/demand.rs | 34 +++++++++++++++++-- .../ui/infinite/infinite-autoderef.stderr | 2 +- src/test/ui/occurs-check-2.stderr | 2 +- src/test/ui/occurs-check.stderr | 2 +- src/test/ui/span/coerce-suggestions.stderr | 2 +- src/test/ui/suggestions/issue-59819.fixed | 22 ++++++++++++ src/test/ui/suggestions/issue-59819.rs | 2 ++ src/test/ui/suggestions/issue-59819.stderr | 9 +++-- src/test/ui/terr-sorts.stderr | 5 ++- 9 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/suggestions/issue-59819.fixed diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 8c1f4aabb1b2b..648a48d8907f1 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -1,6 +1,6 @@ use crate::check::FnCtxt; use rustc::infer::InferOk; -use rustc::traits::{ObligationCause, ObligationCauseCode}; +use rustc::traits::{self, ObligationCause, ObligationCauseCode}; use syntax::util::parser::PREC_POSTFIX; use syntax_pos::Span; @@ -463,7 +463,37 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } } - _ => {} + _ => { + // If neither type is a reference, then check for `Deref` implementations by + // constructing a predicate to prove: `::Output == U` + let deref_trait = self.tcx.lang_items().deref_trait().unwrap(); + let item_def_id = self.tcx.associated_items(deref_trait).next().unwrap().def_id; + let predicate = ty::Predicate::Projection(ty::Binder::bind(ty::ProjectionPredicate { + // `::Output` + projection_ty: ty::ProjectionTy { + // `T` + substs: self.tcx.mk_substs_trait( + checked_ty, + self.fresh_substs_for_item(sp, item_def_id), + ), + // `Deref::Output` + item_def_id, + }, + // `U` + ty: expected, + })); + let obligation = traits::Obligation::new(self.misc(sp), self.param_env, predicate); + if self.infcx.predicate_may_hold(&obligation) { + if let (Ok(code), true) = (cm.span_to_snippet(sp), sp == expr.span) { + let msg = if is_struct_pat_shorthand_field { + format!("{}: *{}", code, code) + } else { + format!("*{}", code) + }; + return Some((sp, "consider dereferencing the type", msg)); + } + } + } } None } diff --git a/src/test/ui/infinite/infinite-autoderef.stderr b/src/test/ui/infinite/infinite-autoderef.stderr index a5cc66f4473f2..04c84bae2dc35 100644 --- a/src/test/ui/infinite/infinite-autoderef.stderr +++ b/src/test/ui/infinite/infinite-autoderef.stderr @@ -5,7 +5,7 @@ LL | x = box x; | ^^^^^ | | | cyclic type of infinite size - | help: try using a conversion method: `box x.to_string()` + | help: consider dereferencing the type: `*box x` error[E0055]: reached the recursion limit while auto-dereferencing `Foo` --> $DIR/infinite-autoderef.rs:25:5 diff --git a/src/test/ui/occurs-check-2.stderr b/src/test/ui/occurs-check-2.stderr index 74e29a5aea728..7b475c3cb227f 100644 --- a/src/test/ui/occurs-check-2.stderr +++ b/src/test/ui/occurs-check-2.stderr @@ -5,7 +5,7 @@ LL | f = box g; | ^^^^^ | | | cyclic type of infinite size - | help: try using a conversion method: `box g.to_string()` + | help: consider dereferencing the type: `*box g` error: aborting due to previous error diff --git a/src/test/ui/occurs-check.stderr b/src/test/ui/occurs-check.stderr index 61ce61b1cbeb6..fe4248a95c90b 100644 --- a/src/test/ui/occurs-check.stderr +++ b/src/test/ui/occurs-check.stderr @@ -5,7 +5,7 @@ LL | f = box f; | ^^^^^ | | | cyclic type of infinite size - | help: try using a conversion method: `box f.to_string()` + | help: consider dereferencing the type: `*box f` error: aborting due to previous error diff --git a/src/test/ui/span/coerce-suggestions.stderr b/src/test/ui/span/coerce-suggestions.stderr index 996d80a07e058..1d8400b20ac03 100644 --- a/src/test/ui/span/coerce-suggestions.stderr +++ b/src/test/ui/span/coerce-suggestions.stderr @@ -44,7 +44,7 @@ LL | f = box f; | ^^^^^ | | | cyclic type of infinite size - | help: try using a conversion method: `box f.to_string()` + | help: consider dereferencing the type: `*box f` error[E0308]: mismatched types --> $DIR/coerce-suggestions.rs:21:9 diff --git a/src/test/ui/suggestions/issue-59819.fixed b/src/test/ui/suggestions/issue-59819.fixed new file mode 100644 index 0000000000000..0bf5d32daf90a --- /dev/null +++ b/src/test/ui/suggestions/issue-59819.fixed @@ -0,0 +1,22 @@ +// run-rustfix + +#![allow(warnings)] + +// Test that suggestion to add `*` characters applies to implementations of `Deref` as well as +// references. + +struct Foo(i32); + +impl std::ops::Deref for Foo { + type Target = i32; + fn deref(&self) -> &i32 { + &self.0 + } +} + +fn main() { + let x = Foo(42); + let y: i32 = *x; //~ ERROR mismatched types + let a = &42; + let b: i32 = *a; //~ ERROR mismatched types +} diff --git a/src/test/ui/suggestions/issue-59819.rs b/src/test/ui/suggestions/issue-59819.rs index b3c4df40161b0..e7e9c7ae2597f 100644 --- a/src/test/ui/suggestions/issue-59819.rs +++ b/src/test/ui/suggestions/issue-59819.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![allow(warnings)] // Test that suggestion to add `*` characters applies to implementations of `Deref` as well as diff --git a/src/test/ui/suggestions/issue-59819.stderr b/src/test/ui/suggestions/issue-59819.stderr index 9c4d613466382..175c39cfc4fac 100644 --- a/src/test/ui/suggestions/issue-59819.stderr +++ b/src/test/ui/suggestions/issue-59819.stderr @@ -1,14 +1,17 @@ error[E0308]: mismatched types - --> $DIR/issue-59819.rs:17:18 + --> $DIR/issue-59819.rs:19:18 | LL | let y: i32 = x; - | ^ expected i32, found struct `Foo` + | ^ + | | + | expected i32, found struct `Foo` + | help: consider dereferencing the type: `*x` | = note: expected type `i32` found type `Foo` error[E0308]: mismatched types - --> $DIR/issue-59819.rs:19:18 + --> $DIR/issue-59819.rs:21:18 | LL | let b: i32 = a; | ^ diff --git a/src/test/ui/terr-sorts.stderr b/src/test/ui/terr-sorts.stderr index 05b9fb43fe1bc..edc042c85ee6e 100644 --- a/src/test/ui/terr-sorts.stderr +++ b/src/test/ui/terr-sorts.stderr @@ -2,7 +2,10 @@ error[E0308]: mismatched types --> $DIR/terr-sorts.rs:10:14 | LL | want_foo(b); - | ^ expected struct `Foo`, found struct `std::boxed::Box` + | ^ + | | + | expected struct `Foo`, found struct `std::boxed::Box` + | help: consider dereferencing the type: `*b` | = note: expected type `Foo` found type `std::boxed::Box` From 7ab1bfd692c2679278c5dc978b04dfb16dfae470 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 22 Apr 2019 10:16:47 +0100 Subject: [PATCH 3/3] Only make suggestion when type is `Copy`. This commit makes the suggestion to dereference when a type implements `Deref` only apply if the dereference would succeed (ie. the type is `Copy`, otherwise a borrow check error would occur). --- src/librustc_typeck/check/demand.rs | 105 +++++++----------- .../ui/infinite/infinite-autoderef.stderr | 2 +- src/test/ui/occurs-check-2.stderr | 2 +- src/test/ui/occurs-check.stderr | 2 +- src/test/ui/span/coerce-suggestions.stderr | 2 +- src/test/ui/suggestions/issue-59819.fixed | 15 ++- src/test/ui/suggestions/issue-59819.rs | 15 ++- src/test/ui/suggestions/issue-59819.stderr | 18 ++- src/test/ui/terr-sorts.stderr | 5 +- 9 files changed, 91 insertions(+), 75 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 648a48d8907f1..6ce03025bf23c 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -324,8 +324,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { sp, ); - match (&expected.sty, &checked_ty.sty) { - (&ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.sty, &check.sty) { + // Check the `expn_info()` to see if this is a macro; if so, it's hard to + // extract the text and make a good suggestion, so don't bother. + let is_macro = sp.ctxt().outer().expn_info().is_some(); + + match (&expr.node, &expected.sty, &checked_ty.sty) { + (_, &ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.sty, &check.sty) { (&ty::Str, &ty::Array(arr, _)) | (&ty::Str, &ty::Slice(arr)) if arr == self.tcx.types.u8 => { if let hir::ExprKind::Lit(_) = expr.node { @@ -352,7 +356,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } _ => {} }, - (&ty::Ref(_, _, mutability), _) => { + (_, &ty::Ref(_, _, mutability), _) => { // Check if it can work when put into a ref. For example: // // ``` @@ -407,65 +411,31 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }); } } - } - (_, &ty::Ref(_, checked, _)) => { + }, + (hir::ExprKind::AddrOf(_, ref expr), _, &ty::Ref(_, checked, _)) if { + self.infcx.can_sub(self.param_env, checked, &expected).is_ok() && !is_macro + } => { // We have `&T`, check if what was expected was `T`. If so, - // we may want to suggest adding a `*`, or removing - // a `&`. - // - // (But, also check the `expn_info()` to see if this is - // a macro; if so, it's hard to extract the text and make a good - // suggestion, so don't bother.) - if self.infcx.can_sub(self.param_env, checked, &expected).is_ok() && - sp.ctxt().outer().expn_info().is_none() { - match expr.node { - // Maybe remove `&`? - hir::ExprKind::AddrOf(_, ref expr) => { - if !cm.span_to_filename(expr.span).is_real() { - if let Ok(code) = cm.span_to_snippet(sp) { - if code.chars().next() == Some('&') { - return Some(( - sp, - "consider removing the borrow", - code[1..].to_string()), - ); - } - } - return None; - } - if let Ok(code) = cm.span_to_snippet(expr.span) { - return Some((sp, "consider removing the borrow", code)); - } - } - - // Maybe add `*`? Only if `T: Copy`. - _ => { - if self.infcx.type_is_copy_modulo_regions(self.param_env, - 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, - ) { - return Some(( - sp, - "consider dereferencing the borrow", - if is_struct_pat_shorthand_field { - format!("{}: *{}", code, code) - } else { - format!("*{}", code) - }, - )); - } - } + // we may want to suggest removing a `&`. + if !cm.span_to_filename(expr.span).is_real() { + if let Ok(code) = cm.span_to_snippet(sp) { + if code.chars().next() == Some('&') { + return Some(( + sp, + "consider removing the borrow", + code[1..].to_string(), + )); } } + return None; } - } - _ => { - // If neither type is a reference, then check for `Deref` implementations by - // constructing a predicate to prove: `::Output == U` + if let Ok(code) = cm.span_to_snippet(expr.span) { + return Some((sp, "consider removing the borrow", code)); + } + }, + _ if sp == expr.span && !is_macro => { + // Check for `Deref` implementations by constructing a predicate to + // prove: `::Output == U` let deref_trait = self.tcx.lang_items().deref_trait().unwrap(); let item_def_id = self.tcx.associated_items(deref_trait).next().unwrap().def_id; let predicate = ty::Predicate::Projection(ty::Binder::bind(ty::ProjectionPredicate { @@ -483,17 +453,28 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { ty: expected, })); let obligation = traits::Obligation::new(self.misc(sp), self.param_env, predicate); - if self.infcx.predicate_may_hold(&obligation) { - if let (Ok(code), true) = (cm.span_to_snippet(sp), sp == expr.span) { - let msg = if is_struct_pat_shorthand_field { + let impls_deref = self.infcx.predicate_may_hold(&obligation); + + // For a suggestion to make sense, the type would need to be `Copy`. + let is_copy = self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp); + + if is_copy && impls_deref { + if let Ok(code) = cm.span_to_snippet(sp) { + let message = if checked_ty.is_region_ptr() { + "consider dereferencing the borrow" + } else { + "consider dereferencing the type" + }; + let suggestion = if is_struct_pat_shorthand_field { format!("{}: *{}", code, code) } else { format!("*{}", code) }; - return Some((sp, "consider dereferencing the type", msg)); + return Some((sp, message, suggestion)); } } } + _ => {} } None } diff --git a/src/test/ui/infinite/infinite-autoderef.stderr b/src/test/ui/infinite/infinite-autoderef.stderr index 04c84bae2dc35..a5cc66f4473f2 100644 --- a/src/test/ui/infinite/infinite-autoderef.stderr +++ b/src/test/ui/infinite/infinite-autoderef.stderr @@ -5,7 +5,7 @@ LL | x = box x; | ^^^^^ | | | cyclic type of infinite size - | help: consider dereferencing the type: `*box x` + | help: try using a conversion method: `box x.to_string()` error[E0055]: reached the recursion limit while auto-dereferencing `Foo` --> $DIR/infinite-autoderef.rs:25:5 diff --git a/src/test/ui/occurs-check-2.stderr b/src/test/ui/occurs-check-2.stderr index 7b475c3cb227f..74e29a5aea728 100644 --- a/src/test/ui/occurs-check-2.stderr +++ b/src/test/ui/occurs-check-2.stderr @@ -5,7 +5,7 @@ LL | f = box g; | ^^^^^ | | | cyclic type of infinite size - | help: consider dereferencing the type: `*box g` + | help: try using a conversion method: `box g.to_string()` error: aborting due to previous error diff --git a/src/test/ui/occurs-check.stderr b/src/test/ui/occurs-check.stderr index fe4248a95c90b..61ce61b1cbeb6 100644 --- a/src/test/ui/occurs-check.stderr +++ b/src/test/ui/occurs-check.stderr @@ -5,7 +5,7 @@ LL | f = box f; | ^^^^^ | | | cyclic type of infinite size - | help: consider dereferencing the type: `*box f` + | help: try using a conversion method: `box f.to_string()` error: aborting due to previous error diff --git a/src/test/ui/span/coerce-suggestions.stderr b/src/test/ui/span/coerce-suggestions.stderr index 1d8400b20ac03..996d80a07e058 100644 --- a/src/test/ui/span/coerce-suggestions.stderr +++ b/src/test/ui/span/coerce-suggestions.stderr @@ -44,7 +44,7 @@ LL | f = box f; | ^^^^^ | | | cyclic type of infinite size - | help: consider dereferencing the type: `*box f` + | help: try using a conversion method: `box f.to_string()` error[E0308]: mismatched types --> $DIR/coerce-suggestions.rs:21:9 diff --git a/src/test/ui/suggestions/issue-59819.fixed b/src/test/ui/suggestions/issue-59819.fixed index 0bf5d32daf90a..644d2a4e41baf 100644 --- a/src/test/ui/suggestions/issue-59819.fixed +++ b/src/test/ui/suggestions/issue-59819.fixed @@ -7,9 +7,18 @@ struct Foo(i32); +struct Bar(String); + impl std::ops::Deref for Foo { type Target = i32; - fn deref(&self) -> &i32 { + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl std::ops::Deref for Bar { + type Target = String; + fn deref(&self) -> &Self::Target { &self.0 } } @@ -19,4 +28,8 @@ fn main() { let y: i32 = *x; //~ ERROR mismatched types let a = &42; let b: i32 = *a; //~ ERROR mismatched types + + // Do not make a suggestion when adding a `*` wouldn't actually fix the issue: + let f = Bar("bar".to_string()); + let g: String = f.to_string(); //~ ERROR mismatched types } diff --git a/src/test/ui/suggestions/issue-59819.rs b/src/test/ui/suggestions/issue-59819.rs index e7e9c7ae2597f..8e8ff8372e808 100644 --- a/src/test/ui/suggestions/issue-59819.rs +++ b/src/test/ui/suggestions/issue-59819.rs @@ -7,9 +7,18 @@ struct Foo(i32); +struct Bar(String); + impl std::ops::Deref for Foo { type Target = i32; - fn deref(&self) -> &i32 { + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl std::ops::Deref for Bar { + type Target = String; + fn deref(&self) -> &Self::Target { &self.0 } } @@ -19,4 +28,8 @@ fn main() { let y: i32 = x; //~ ERROR mismatched types let a = &42; let b: i32 = a; //~ ERROR mismatched types + + // Do not make a suggestion when adding a `*` wouldn't actually fix the issue: + let f = Bar("bar".to_string()); + let g: String = f; //~ ERROR mismatched types } diff --git a/src/test/ui/suggestions/issue-59819.stderr b/src/test/ui/suggestions/issue-59819.stderr index 175c39cfc4fac..66898115cbd6d 100644 --- a/src/test/ui/suggestions/issue-59819.stderr +++ b/src/test/ui/suggestions/issue-59819.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/issue-59819.rs:19:18 + --> $DIR/issue-59819.rs:28:18 | LL | let y: i32 = x; | ^ @@ -11,7 +11,7 @@ LL | let y: i32 = x; found type `Foo` error[E0308]: mismatched types - --> $DIR/issue-59819.rs:21:18 + --> $DIR/issue-59819.rs:30:18 | LL | let b: i32 = a; | ^ @@ -22,6 +22,18 @@ LL | let b: i32 = a; = note: expected type `i32` found type `&{integer}` -error: aborting due to 2 previous errors +error[E0308]: mismatched types + --> $DIR/issue-59819.rs:34:21 + | +LL | let g: String = f; + | ^ + | | + | expected struct `std::string::String`, found struct `Bar` + | help: try using a conversion method: `f.to_string()` + | + = note: expected type `std::string::String` + found type `Bar` + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/terr-sorts.stderr b/src/test/ui/terr-sorts.stderr index edc042c85ee6e..05b9fb43fe1bc 100644 --- a/src/test/ui/terr-sorts.stderr +++ b/src/test/ui/terr-sorts.stderr @@ -2,10 +2,7 @@ error[E0308]: mismatched types --> $DIR/terr-sorts.rs:10:14 | LL | want_foo(b); - | ^ - | | - | expected struct `Foo`, found struct `std::boxed::Box` - | help: consider dereferencing the type: `*b` + | ^ expected struct `Foo`, found struct `std::boxed::Box` | = note: expected type `Foo` found type `std::boxed::Box`