Skip to content

Commit 5ce3f56

Browse files
compiler-errorscompiler-errors
committed
Make deref suggestion better
1 parent 6a70556 commit 5ce3f56

File tree

7 files changed

+214
-60
lines changed

7 files changed

+214
-60
lines changed

‎compiler/rustc_typeck/src/check/demand.rs‎

Lines changed: 92 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
566566
expr: &hir::Expr<'tcx>,
567567
checked_ty: Ty<'tcx>,
568568
expected: Ty<'tcx>,
569-
) -> Option<(Span, &'static str, String, Applicability, bool /* verbose */)> {
569+
) -> Option<(Span, String, String, Applicability, bool /* verbose */)> {
570570
let sess = self.sess();
571571
let sp = expr.span;
572572

@@ -593,7 +593,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
593593
let pos = sp.lo() + BytePos(1);
594594
return Some((
595595
sp.with_hi(pos),
596-
"consider removing the leading `b`",
596+
"consider removing the leading `b`".to_string(),
597597
String::new(),
598598
Applicability::MachineApplicable,
599599
true,
@@ -608,7 +608,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
608608
if replace_prefix(&src, "\"", "b\"").is_some() {
609609
return Some((
610610
sp.shrink_to_lo(),
611-
"consider adding a leading `b`",
611+
"consider adding a leading `b`".to_string(),
612612
"b".to_string(),
613613
Applicability::MachineApplicable,
614614
true,
@@ -669,7 +669,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
669669
if let Some(sugg) = self.can_use_as_ref(expr) {
670670
return Some((
671671
sugg.0,
672-
sugg.1,
672+
sugg.1.to_string(),
673673
sugg.2,
674674
Applicability::MachineApplicable,
675675
false,
@@ -697,7 +697,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
697697
return Some((
698698
left_expr.span.shrink_to_lo(),
699699
"consider dereferencing here to assign to the mutable \
700-
borrowed piece of memory",
700+
borrowed piece of memory"
701+
.to_string(),
701702
"*".to_string(),
702703
Applicability::MachineApplicable,
703704
true,
@@ -709,14 +710,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
709710
return Some(match mutability {
710711
hir::Mutability::Mut => (
711712
sp,
712-
"consider mutably borrowing here",
713+
"consider mutably borrowing here".to_string(),
713714
format!("{}&mut {}", prefix, sugg_expr),
714715
Applicability::MachineApplicable,
715716
false,
716717
),
717718
hir::Mutability::Not => (
718719
sp,
719-
"consider borrowing here",
720+
"consider borrowing here".to_string(),
720721
format!("{}&{}", prefix, sugg_expr),
721722
Applicability::MachineApplicable,
722723
false,
@@ -745,7 +746,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
745746
if sm.span_to_snippet(call_span).is_ok() {
746747
return Some((
747748
sp.with_hi(call_span.lo()),
748-
"consider removing the borrow",
749+
"consider removing the borrow".to_string(),
749750
String::new(),
750751
Applicability::MachineApplicable,
751752
true,
@@ -758,7 +759,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
758759
if sm.span_to_snippet(expr.span).is_ok() {
759760
return Some((
760761
sp.with_hi(expr.span.lo()),
761-
"consider removing the borrow",
762+
"consider removing the borrow".to_string(),
762763
String::new(),
763764
Applicability::MachineApplicable,
764765
true,
@@ -824,7 +825,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
824825
} {
825826
return Some((
826827
span,
827-
"consider dereferencing",
828+
"consider dereferencing".to_string(),
828829
src,
829830
applicability,
830831
true,
@@ -835,60 +836,93 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
835836
}
836837
}
837838
_ if sp == expr.span => {
838-
if let Some(steps) = self.deref_steps(checked_ty, expected) {
839-
let expr = expr.peel_blocks();
839+
if let Some(mut steps) = self.deref_steps(checked_ty, expected) {
840+
let mut expr = expr.peel_blocks();
841+
let mut prefix_span = expr.span.shrink_to_lo();
842+
let mut remove = String::new();
840843

841-
if steps == 1 {
844+
// Try peeling off any existing `&` and `&mut` to reach our target type
845+
while steps > 0 {
842846
if let hir::ExprKind::AddrOf(_, mutbl, inner) = expr.kind {
843847
// If the expression has `&`, removing it would fix the error
844-
let prefix_span = expr.span.with_hi(inner.span.lo());
845-
let message = match mutbl {
846-
hir::Mutability::Not => "consider removing the `&`",
847-
hir::Mutability::Mut => "consider removing the `&mut`",
848+
prefix_span = prefix_span.with_hi(inner.span.lo());
849+
expr = inner;
850+
remove += match mutbl {
851+
hir::Mutability::Not => "&",
852+
hir::Mutability::Mut => "&mut ",
848853
};
849-
let suggestion = String::new();
850-
return Some((
851-
prefix_span,
852-
message,
853-
suggestion,
854-
Applicability::MachineApplicable,
855-
false,
856-
));
854+
steps -= 1;
855+
} else {
856+
break;
857857
}
858-
859-
// For this suggestion to make sense, the type would need to be `Copy`,
860-
// or we have to be moving out of a `Box<T>`
861-
if self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp)
862-
|| checked_ty.is_box()
863-
{
864-
let message = if checked_ty.is_box() {
865-
"consider unboxing the value"
866-
} else if checked_ty.is_region_ptr() {
867-
"consider dereferencing the borrow"
868-
} else {
869-
"consider dereferencing the type"
870-
};
871-
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
872-
Some(ident) => format!("{}: ", ident),
873-
None => String::new(),
874-
};
875-
let (span, suggestion) = if self.is_else_if_block(expr) {
876-
// Don't suggest nonsense like `else *if`
877-
return None;
878-
} else if let Some(expr) = self.maybe_get_block_expr(expr) {
879-
// prefix should be empty here..
880-
(expr.span.shrink_to_lo(), "*".to_string())
858+
}
859+
// If we've reached our target type with just removing `&`, then just print now.
860+
if steps == 0 {
861+
return Some((
862+
prefix_span,
863+
format!("consider removing the `{}`", remove.trim()),
864+
String::new(),
865+
// Do not remove `&&` to get to bool, because it might be something like
866+
// { a } && b, which we have a separate fixup suggestion that is more
867+
// likely correct...
868+
if remove.trim() == "&&" && expected == self.tcx.types.bool {
869+
Applicability::MaybeIncorrect
881870
} else {
882-
(expr.span.shrink_to_lo(), format!("{}*", prefix))
883-
};
884-
return Some((
885-
span,
886-
message,
887-
suggestion,
888-
Applicability::MachineApplicable,
889-
true,
890-
));
891-
}
871+
Applicability::MachineApplicable
872+
},
873+
true,
874+
));
875+
}
876+
877+
// For this suggestion to make sense, the type would need to be `Copy`,
878+
// or we have to be moving out of a `Box<T>`
879+
if self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp)
880+
// FIXME(compiler-errors): We can actually do this if the checked_ty is
881+
// `steps` layers of boxes, not just one, but this is easier and most likely.
882+
|| (checked_ty.is_box() && steps == 1)
883+
{
884+
let deref_kind = if checked_ty.is_box() {
885+
"unboxing the value"
886+
} else if checked_ty.is_region_ptr() {
887+
"dereferencing the borrow"
888+
} else {
889+
"dereferencing the type"
890+
};
891+
892+
// Suggest removing `&` if we have removed any, otherwise suggest just
893+
// dereferencing the remaining number of steps.
894+
let message = if remove.is_empty() {
895+
format!("consider {}", deref_kind)
896+
} else {
897+
format!(
898+
"consider removing the `{}` and {} instead",
899+
remove.trim(),
900+
deref_kind
901+
)
902+
};
903+
904+
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
905+
Some(ident) => format!("{}: ", ident),
906+
None => String::new(),
907+
};
908+
909+
let (span, suggestion) = if self.is_else_if_block(expr) {
910+
// Don't suggest nonsense like `else *if`
911+
return None;
912+
} else if let Some(expr) = self.maybe_get_block_expr(expr) {
913+
// prefix should be empty here..
914+
(expr.span.shrink_to_lo(), "*".to_string())
915+
} else {
916+
(prefix_span, format!("{}{}", prefix, "*".repeat(steps)))
917+
};
918+
919+
return Some((
920+
span,
921+
message,
922+
suggestion,
923+
Applicability::MachineApplicable,
924+
true,
925+
));
892926
}
893927
}
894928
}

‎compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
218218
self.check_ref(expr, found, expected)
219219
{
220220
if verbose {
221-
err.span_suggestion_verbose(sp, msg, suggestion, applicability);
221+
err.span_suggestion_verbose(sp, &msg, suggestion, applicability);
222222
} else {
223-
err.span_suggestion(sp, msg, suggestion, applicability);
223+
err.span_suggestion(sp, &msg, suggestion, applicability);
224224
}
225225
} else if let (ty::FnDef(def_id, ..), true) =
226226
(&found.kind(), self.suggest_fn_call(err, expr, expected, found))

‎src/test/ui/parser/expr-as-stmt-2.stderr‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ LL | / &&
3636
LL | | if let Some(y) = a { true } else { false }
3737
| |______________________________________________^ expected `bool`, found `&&bool`
3838
|
39+
help: consider removing the `&&`
40+
|
41+
LL - &&
42+
LL + if let Some(y) = a { true } else { false }
43+
|
3944
help: parentheses are required to parse this as an expression
4045
|
4146
LL | (if let Some(x) = a { true } else { false })

‎src/test/ui/parser/expr-as-stmt.stderr‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,11 @@ LL | fn revenge_from_mars() -> bool {
170170
LL | { true } && { true }
171171
| ^^^^^^^^^^^ expected `bool`, found `&&bool`
172172
|
173+
help: consider removing the `&&`
174+
|
175+
LL - { true } && { true }
176+
LL + { true } { true }
177+
|
173178
help: parentheses are required to parse this as an expression
174179
|
175180
LL | ({ true }) && { true }

‎src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,12 @@ error[E0308]: mismatched types
676676
|
677677
LL | if let Range { start: true, end } = t..&&false {}
678678
| ^^^^^^^ expected `bool`, found `&&bool`
679+
|
680+
help: consider removing the `&&`
681+
|
682+
LL - if let Range { start: true, end } = t..&&false {}
683+
LL + if let Range { start: true, end } = t..false {}
684+
|
679685

680686
error[E0308]: mismatched types
681687
--> $DIR/disallowed-positions.rs:83:8
@@ -866,6 +872,12 @@ error[E0308]: mismatched types
866872
|
867873
LL | while let Range { start: true, end } = t..&&false {}
868874
| ^^^^^^^ expected `bool`, found `&&bool`
875+
|
876+
help: consider removing the `&&`
877+
|
878+
LL - while let Range { start: true, end } = t..&&false {}
879+
LL + while let Range { start: true, end } = t..false {}
880+
|
869881

870882
error[E0308]: mismatched types
871883
--> $DIR/disallowed-positions.rs:147:11

‎src/test/ui/typeck/deref-multi.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
fn a(x: &&i32) -> i32 {
2+
x
3+
//~^ ERROR mismatched types
4+
}
5+
6+
fn a2(x: &&&&&i32) -> i32 {
7+
x
8+
//~^ ERROR mismatched types
9+
}
10+
11+
fn b(x: &i32) -> i32 {
12+
&x
13+
//~^ ERROR mismatched types
14+
}
15+
16+
fn c(x: Box<i32>) -> i32 {
17+
&x
18+
//~^ ERROR mismatched types
19+
}
20+
21+
fn d(x: std::sync::Mutex<&i32>) -> i32 {
22+
x.lock().unwrap()
23+
//~^ ERROR mismatched types
24+
}
25+
26+
fn main() {}

‎src/test/ui/typeck/deref-multi.stderr‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/deref-multi.rs:2:5
3+
|
4+
LL | fn a(x: &&i32) -> i32 {
5+
| --- expected `i32` because of return type
6+
LL | x
7+
| ^ expected `i32`, found `&&i32`
8+
|
9+
help: consider dereferencing the borrow
10+
|
11+
LL | **x
12+
| ++
13+
14+
error[E0308]: mismatched types
15+
--> $DIR/deref-multi.rs:7:5
16+
|
17+
LL | fn a2(x: &&&&&i32) -> i32 {
18+
| --- expected `i32` because of return type
19+
LL | x
20+
| ^ expected `i32`, found `&&&&&i32`
21+
|
22+
help: consider dereferencing the borrow
23+
|
24+
LL | *****x
25+
| +++++
26+
27+
error[E0308]: mismatched types
28+
--> $DIR/deref-multi.rs:12:5
29+
|
30+
LL | fn b(x: &i32) -> i32 {
31+
| --- expected `i32` because of return type
32+
LL | &x
33+
| ^^ expected `i32`, found `&&i32`
34+
|
35+
help: consider removing the `&` and dereferencing the borrow instead
36+
|
37+
LL | *x
38+
| ~
39+
40+
error[E0308]: mismatched types
41+
--> $DIR/deref-multi.rs:17:5
42+
|
43+
LL | fn c(x: Box<i32>) -> i32 {
44+
| --- expected `i32` because of return type
45+
LL | &x
46+
| ^^ expected `i32`, found `&Box<i32>`
47+
|
48+
= note: expected type `i32`
49+
found reference `&Box<i32>`
50+
help: consider removing the `&` and dereferencing the borrow instead
51+
|
52+
LL | *x
53+
| ~
54+
55+
error[E0308]: mismatched types
56+
--> $DIR/deref-multi.rs:22:5
57+
|
58+
LL | fn d(x: std::sync::Mutex<&i32>) -> i32 {
59+
| --- expected `i32` because of return type
60+
LL | x.lock().unwrap()
61+
| ^^^^^^^^^^^^^^^^^ expected `i32`, found struct `MutexGuard`
62+
|
63+
= note: expected type `i32`
64+
found struct `MutexGuard<'_, &i32>`
65+
help: consider dereferencing the type
66+
|
67+
LL | **x.lock().unwrap()
68+
| ++
69+
70+
error: aborting due to 5 previous errors
71+
72+
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)