Skip to content

Commit 7ab1bfd

Browse files
davidtwcodavidtwco
committed
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).
1 parent fd95ba3 commit 7ab1bfd

File tree

9 files changed

+91
-75
lines changed

9 files changed

+91
-75
lines changed

Diff for: ‎src/librustc_typeck/check/demand.rs

+43-62
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
324324
sp,
325325
);
326326

327-
match (&expected.sty, &checked_ty.sty) {
328-
(&ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.sty, &check.sty) {
327+
// Check the `expn_info()` to see if this is a macro; if so, it's hard to
328+
// extract the text and make a good suggestion, so don't bother.
329+
let is_macro = sp.ctxt().outer().expn_info().is_some();
330+
331+
match (&expr.node, &expected.sty, &checked_ty.sty) {
332+
(_, &ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.sty, &check.sty) {
329333
(&ty::Str, &ty::Array(arr, _)) |
330334
(&ty::Str, &ty::Slice(arr)) if arr == self.tcx.types.u8 => {
331335
if let hir::ExprKind::Lit(_) = expr.node {
@@ -352,7 +356,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
352356
}
353357
_ => {}
354358
},
355-
(&ty::Ref(_, _, mutability), _) => {
359+
(_, &ty::Ref(_, _, mutability), _) => {
356360
// Check if it can work when put into a ref. For example:
357361
//
358362
// ```
@@ -407,65 +411,31 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
407411
});
408412
}
409413
}
410-
}
411-
(_, &ty::Ref(_, checked, _)) => {
414+
},
415+
(hir::ExprKind::AddrOf(_, ref expr), _, &ty::Ref(_, checked, _)) if {
416+
self.infcx.can_sub(self.param_env, checked, &expected).is_ok() && !is_macro
417+
} => {
412418
// We have `&T`, check if what was expected was `T`. If so,
413-
// we may want to suggest adding a `*`, or removing
414-
// a `&`.
415-
//
416-
// (But, also check the `expn_info()` to see if this is
417-
// a macro; if so, it's hard to extract the text and make a good
418-
// suggestion, so don't bother.)
419-
if self.infcx.can_sub(self.param_env, checked, &expected).is_ok() &&
420-
sp.ctxt().outer().expn_info().is_none() {
421-
match expr.node {
422-
// Maybe remove `&`?
423-
hir::ExprKind::AddrOf(_, ref expr) => {
424-
if !cm.span_to_filename(expr.span).is_real() {
425-
if let Ok(code) = cm.span_to_snippet(sp) {
426-
if code.chars().next() == Some('&') {
427-
return Some((
428-
sp,
429-
"consider removing the borrow",
430-
code[1..].to_string()),
431-
);
432-
}
433-
}
434-
return None;
435-
}
436-
if let Ok(code) = cm.span_to_snippet(expr.span) {
437-
return Some((sp, "consider removing the borrow", code));
438-
}
439-
}
440-
441-
// Maybe add `*`? Only if `T: Copy`.
442-
_ => {
443-
if self.infcx.type_is_copy_modulo_regions(self.param_env,
444-
checked,
445-
sp) {
446-
// do not suggest if the span comes from a macro (#52783)
447-
if let (Ok(code), true) = (
448-
cm.span_to_snippet(sp),
449-
sp == expr.span,
450-
) {
451-
return Some((
452-
sp,
453-
"consider dereferencing the borrow",
454-
if is_struct_pat_shorthand_field {
455-
format!("{}: *{}", code, code)
456-
} else {
457-
format!("*{}", code)
458-
},
459-
));
460-
}
461-
}
419+
// we may want to suggest removing a `&`.
420+
if !cm.span_to_filename(expr.span).is_real() {
421+
if let Ok(code) = cm.span_to_snippet(sp) {
422+
if code.chars().next() == Some('&') {
423+
return Some((
424+
sp,
425+
"consider removing the borrow",
426+
code[1..].to_string(),
427+
));
462428
}
463429
}
430+
return None;
464431
}
465-
}
466-
_ => {
467-
// If neither type is a reference, then check for `Deref` implementations by
468-
// constructing a predicate to prove: `<T as Deref>::Output == U`
432+
if let Ok(code) = cm.span_to_snippet(expr.span) {
433+
return Some((sp, "consider removing the borrow", code));
434+
}
435+
},
436+
_ if sp == expr.span && !is_macro => {
437+
// Check for `Deref` implementations by constructing a predicate to
438+
// prove: `<T as Deref>::Output == U`
469439
let deref_trait = self.tcx.lang_items().deref_trait().unwrap();
470440
let item_def_id = self.tcx.associated_items(deref_trait).next().unwrap().def_id;
471441
let predicate = ty::Predicate::Projection(ty::Binder::bind(ty::ProjectionPredicate {
@@ -483,17 +453,28 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
483453
ty: expected,
484454
}));
485455
let obligation = traits::Obligation::new(self.misc(sp), self.param_env, predicate);
486-
if self.infcx.predicate_may_hold(&obligation) {
487-
if let (Ok(code), true) = (cm.span_to_snippet(sp), sp == expr.span) {
488-
let msg = if is_struct_pat_shorthand_field {
456+
let impls_deref = self.infcx.predicate_may_hold(&obligation);
457+
458+
// For a suggestion to make sense, the type would need to be `Copy`.
459+
let is_copy = self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp);
460+
461+
if is_copy && impls_deref {
462+
if let Ok(code) = cm.span_to_snippet(sp) {
463+
let message = if checked_ty.is_region_ptr() {
464+
"consider dereferencing the borrow"
465+
} else {
466+
"consider dereferencing the type"
467+
};
468+
let suggestion = if is_struct_pat_shorthand_field {
489469
format!("{}: *{}", code, code)
490470
} else {
491471
format!("*{}", code)
492472
};
493-
return Some((sp, "consider dereferencing the type", msg));
473+
return Some((sp, message, suggestion));
494474
}
495475
}
496476
}
477+
_ => {}
497478
}
498479
None
499480
}

Diff for: ‎src/test/ui/infinite/infinite-autoderef.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | x = box x;
55
| ^^^^^
66
| |
77
| cyclic type of infinite size
8-
| help: consider dereferencing the type: `*box x`
8+
| help: try using a conversion method: `box x.to_string()`
99

1010
error[E0055]: reached the recursion limit while auto-dereferencing `Foo`
1111
--> $DIR/infinite-autoderef.rs:25:5

Diff for: ‎src/test/ui/occurs-check-2.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | f = box g;
55
| ^^^^^
66
| |
77
| cyclic type of infinite size
8-
| help: consider dereferencing the type: `*box g`
8+
| help: try using a conversion method: `box g.to_string()`
99

1010
error: aborting due to previous error
1111

Diff for: ‎src/test/ui/occurs-check.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | f = box f;
55
| ^^^^^
66
| |
77
| cyclic type of infinite size
8-
| help: consider dereferencing the type: `*box f`
8+
| help: try using a conversion method: `box f.to_string()`
99

1010
error: aborting due to previous error
1111

Diff for: ‎src/test/ui/span/coerce-suggestions.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ LL | f = box f;
4444
| ^^^^^
4545
| |
4646
| cyclic type of infinite size
47-
| help: consider dereferencing the type: `*box f`
47+
| help: try using a conversion method: `box f.to_string()`
4848

4949
error[E0308]: mismatched types
5050
--> $DIR/coerce-suggestions.rs:21:9

Diff for: ‎src/test/ui/suggestions/issue-59819.fixed

+14-1
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,18 @@
77

88
struct Foo(i32);
99

10+
struct Bar(String);
11+
1012
impl std::ops::Deref for Foo {
1113
type Target = i32;
12-
fn deref(&self) -> &i32 {
14+
fn deref(&self) -> &Self::Target {
15+
&self.0
16+
}
17+
}
18+
19+
impl std::ops::Deref for Bar {
20+
type Target = String;
21+
fn deref(&self) -> &Self::Target {
1322
&self.0
1423
}
1524
}
@@ -19,4 +28,8 @@ fn main() {
1928
let y: i32 = *x; //~ ERROR mismatched types
2029
let a = &42;
2130
let b: i32 = *a; //~ ERROR mismatched types
31+
32+
// Do not make a suggestion when adding a `*` wouldn't actually fix the issue:
33+
let f = Bar("bar".to_string());
34+
let g: String = f.to_string(); //~ ERROR mismatched types
2235
}

Diff for: ‎src/test/ui/suggestions/issue-59819.rs

+14-1
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,18 @@
77

88
struct Foo(i32);
99

10+
struct Bar(String);
11+
1012
impl std::ops::Deref for Foo {
1113
type Target = i32;
12-
fn deref(&self) -> &i32 {
14+
fn deref(&self) -> &Self::Target {
15+
&self.0
16+
}
17+
}
18+
19+
impl std::ops::Deref for Bar {
20+
type Target = String;
21+
fn deref(&self) -> &Self::Target {
1322
&self.0
1423
}
1524
}
@@ -19,4 +28,8 @@ fn main() {
1928
let y: i32 = x; //~ ERROR mismatched types
2029
let a = &42;
2130
let b: i32 = a; //~ ERROR mismatched types
31+
32+
// Do not make a suggestion when adding a `*` wouldn't actually fix the issue:
33+
let f = Bar("bar".to_string());
34+
let g: String = f; //~ ERROR mismatched types
2235
}

Diff for: ‎src/test/ui/suggestions/issue-59819.stderr

+15-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0308]: mismatched types
2-
--> $DIR/issue-59819.rs:19:18
2+
--> $DIR/issue-59819.rs:28:18
33
|
44
LL | let y: i32 = x;
55
| ^
@@ -11,7 +11,7 @@ LL | let y: i32 = x;
1111
found type `Foo`
1212

1313
error[E0308]: mismatched types
14-
--> $DIR/issue-59819.rs:21:18
14+
--> $DIR/issue-59819.rs:30:18
1515
|
1616
LL | let b: i32 = a;
1717
| ^
@@ -22,6 +22,18 @@ LL | let b: i32 = a;
2222
= note: expected type `i32`
2323
found type `&{integer}`
2424

25-
error: aborting due to 2 previous errors
25+
error[E0308]: mismatched types
26+
--> $DIR/issue-59819.rs:34:21
27+
|
28+
LL | let g: String = f;
29+
| ^
30+
| |
31+
| expected struct `std::string::String`, found struct `Bar`
32+
| help: try using a conversion method: `f.to_string()`
33+
|
34+
= note: expected type `std::string::String`
35+
found type `Bar`
36+
37+
error: aborting due to 3 previous errors
2638

2739
For more information about this error, try `rustc --explain E0308`.

Diff for: ‎src/test/ui/terr-sorts.stderr

+1-4
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,7 @@ error[E0308]: mismatched types
22
--> $DIR/terr-sorts.rs:10:14
33
|
44
LL | want_foo(b);
5-
| ^
6-
| |
7-
| expected struct `Foo`, found struct `std::boxed::Box`
8-
| help: consider dereferencing the type: `*b`
5+
| ^ expected struct `Foo`, found struct `std::boxed::Box`
96
|
107
= note: expected type `Foo`
118
found type `std::boxed::Box<Foo>`

0 commit comments

Comments
 (0)