Skip to content

Commit

Permalink
Only make suggestion when type is Copy.
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
davidtwco committed Apr 22, 2019
1 parent fd95ba3 commit 7ab1bfd
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 75 deletions.
105 changes: 43 additions & 62 deletions src/librustc_typeck/check/demand.rs
Expand Up @@ -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 {
Expand All @@ -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:
//
// ```
Expand Down Expand Up @@ -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: `<T as Deref>::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: `<T as Deref>::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 {
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/infinite/infinite-autoderef.stderr
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/occurs-check-2.stderr
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/occurs-check.stderr
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/span/coerce-suggestions.stderr
Expand Up @@ -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
Expand Down
15 changes: 14 additions & 1 deletion src/test/ui/suggestions/issue-59819.fixed
Expand Up @@ -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
}
}
Expand All @@ -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
}
15 changes: 14 additions & 1 deletion src/test/ui/suggestions/issue-59819.rs
Expand Up @@ -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
}
}
Expand All @@ -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
}
18 changes: 15 additions & 3 deletions 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;
| ^
Expand All @@ -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;
| ^
Expand All @@ -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`.
5 changes: 1 addition & 4 deletions src/test/ui/terr-sorts.stderr
Expand Up @@ -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<Foo>`
Expand Down

0 comments on commit 7ab1bfd

Please sign in to comment.