Rollup merge of #91545 - compiler-errors:deref-suggestion-improvements, r=estebank

Generalize "remove `&`"  and "add `*`" suggestions to more than one deref

Suggest removing more than one `&` and `&mut`, along with suggesting adding more than one `*` (or a combination of the two).

r? `@estebank`
(since you're experienced with these types of suggestions, feel free to reassign)
This commit is contained in:
Dylan DPC 2022-03-01 03:41:46 +01:00 committed by GitHub
commit 4bd40d67d8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 214 additions and 60 deletions

View File

@ -566,7 +566,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr: &hir::Expr<'tcx>,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>,
) -> Option<(Span, &'static str, String, Applicability, bool /* verbose */)> {
) -> Option<(Span, String, String, Applicability, bool /* verbose */)> {
let sess = self.sess();
let sp = expr.span;
@ -594,7 +594,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let pos = sp.lo() + BytePos(1);
return Some((
sp.with_hi(pos),
"consider removing the leading `b`",
"consider removing the leading `b`".to_string(),
String::new(),
Applicability::MachineApplicable,
true,
@ -608,7 +608,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
{
return Some((
sp.shrink_to_lo(),
"consider adding a leading `b`",
"consider adding a leading `b`".to_string(),
"b".to_string(),
Applicability::MachineApplicable,
true,
@ -668,7 +668,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let Some(sugg) = self.can_use_as_ref(expr) {
return Some((
sugg.0,
sugg.1,
sugg.1.to_string(),
sugg.2,
Applicability::MachineApplicable,
false,
@ -696,7 +696,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return Some((
left_expr.span.shrink_to_lo(),
"consider dereferencing here to assign to the mutable \
borrowed piece of memory",
borrowed piece of memory"
.to_string(),
"*".to_string(),
Applicability::MachineApplicable,
true,
@ -708,14 +709,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return Some(match mutability {
hir::Mutability::Mut => (
sp,
"consider mutably borrowing here",
"consider mutably borrowing here".to_string(),
format!("{}&mut {}", prefix, sugg_expr),
Applicability::MachineApplicable,
false,
),
hir::Mutability::Not => (
sp,
"consider borrowing here",
"consider borrowing here".to_string(),
format!("{}&{}", prefix, sugg_expr),
Applicability::MachineApplicable,
false,
@ -744,7 +745,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if sm.span_to_snippet(call_span).is_ok() {
return Some((
sp.with_hi(call_span.lo()),
"consider removing the borrow",
"consider removing the borrow".to_string(),
String::new(),
Applicability::MachineApplicable,
true,
@ -757,7 +758,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if sm.span_to_snippet(expr.span).is_ok() {
return Some((
sp.with_hi(expr.span.lo()),
"consider removing the borrow",
"consider removing the borrow".to_string(),
String::new(),
Applicability::MachineApplicable,
true,
@ -823,7 +824,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} {
return Some((
span,
"consider dereferencing",
"consider dereferencing".to_string(),
src,
applicability,
true,
@ -834,60 +835,93 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
_ if sp == expr.span => {
if let Some(steps) = self.deref_steps(checked_ty, expected) {
let expr = expr.peel_blocks();
if let Some(mut steps) = self.deref_steps(checked_ty, expected) {
let mut expr = expr.peel_blocks();
let mut prefix_span = expr.span.shrink_to_lo();
let mut remove = String::new();
if steps == 1 {
// Try peeling off any existing `&` and `&mut` to reach our target type
while steps > 0 {
if let hir::ExprKind::AddrOf(_, mutbl, inner) = expr.kind {
// If the expression has `&`, removing it would fix the error
let prefix_span = expr.span.with_hi(inner.span.lo());
let message = match mutbl {
hir::Mutability::Not => "consider removing the `&`",
hir::Mutability::Mut => "consider removing the `&mut`",
prefix_span = prefix_span.with_hi(inner.span.lo());
expr = inner;
remove += match mutbl {
hir::Mutability::Not => "&",
hir::Mutability::Mut => "&mut ",
};
let suggestion = String::new();
return Some((
prefix_span,
message,
suggestion,
Applicability::MachineApplicable,
false,
));
steps -= 1;
} else {
break;
}
}
// If we've reached our target type with just removing `&`, then just print now.
if steps == 0 {
return Some((
prefix_span,
format!("consider removing the `{}`", remove.trim()),
String::new(),
// Do not remove `&&` to get to bool, because it might be something like
// { a } && b, which we have a separate fixup suggestion that is more
// likely correct...
if remove.trim() == "&&" && expected == self.tcx.types.bool {
Applicability::MaybeIncorrect
} else {
Applicability::MachineApplicable
},
true,
));
}
// For this suggestion to make sense, the type would need to be `Copy`,
// or we have to be moving out of a `Box<T>`
if self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp)
|| checked_ty.is_box()
{
let message = if checked_ty.is_box() {
"consider unboxing the value"
} else if checked_ty.is_region_ptr() {
"consider dereferencing the borrow"
} else {
"consider dereferencing the type"
};
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!("{}: ", ident),
None => String::new(),
};
let (span, suggestion) = if self.is_else_if_block(expr) {
// Don't suggest nonsense like `else *if`
return None;
} else if let Some(expr) = self.maybe_get_block_expr(expr) {
// prefix should be empty here..
(expr.span.shrink_to_lo(), "*".to_string())
} else {
(expr.span.shrink_to_lo(), format!("{}*", prefix))
};
return Some((
span,
message,
suggestion,
Applicability::MachineApplicable,
true,
));
}
// For this suggestion to make sense, the type would need to be `Copy`,
// or we have to be moving out of a `Box<T>`
if self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp)
// FIXME(compiler-errors): We can actually do this if the checked_ty is
// `steps` layers of boxes, not just one, but this is easier and most likely.
|| (checked_ty.is_box() && steps == 1)
{
let deref_kind = if checked_ty.is_box() {
"unboxing the value"
} else if checked_ty.is_region_ptr() {
"dereferencing the borrow"
} else {
"dereferencing the type"
};
// Suggest removing `&` if we have removed any, otherwise suggest just
// dereferencing the remaining number of steps.
let message = if remove.is_empty() {
format!("consider {}", deref_kind)
} else {
format!(
"consider removing the `{}` and {} instead",
remove.trim(),
deref_kind
)
};
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!("{}: ", ident),
None => String::new(),
};
let (span, suggestion) = if self.is_else_if_block(expr) {
// Don't suggest nonsense like `else *if`
return None;
} else if let Some(expr) = self.maybe_get_block_expr(expr) {
// prefix should be empty here..
(expr.span.shrink_to_lo(), "*".to_string())
} else {
(prefix_span, format!("{}{}", prefix, "*".repeat(steps)))
};
return Some((
span,
message,
suggestion,
Applicability::MachineApplicable,
true,
));
}
}
}

View File

@ -218,9 +218,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.check_ref(expr, found, expected)
{
if verbose {
err.span_suggestion_verbose(sp, msg, suggestion, applicability);
err.span_suggestion_verbose(sp, &msg, suggestion, applicability);
} else {
err.span_suggestion(sp, msg, suggestion, applicability);
err.span_suggestion(sp, &msg, suggestion, applicability);
}
} else if let (ty::FnDef(def_id, ..), true) =
(&found.kind(), self.suggest_fn_call(err, expr, expected, found))

View File

@ -36,6 +36,11 @@ LL | / &&
LL | | if let Some(y) = a { true } else { false }
| |______________________________________________^ expected `bool`, found `&&bool`
|
help: consider removing the `&&`
|
LL - &&
LL + if let Some(y) = a { true } else { false }
|
help: parentheses are required to parse this as an expression
|
LL | (if let Some(x) = a { true } else { false })

View File

@ -170,6 +170,11 @@ LL | fn revenge_from_mars() -> bool {
LL | { true } && { true }
| ^^^^^^^^^^^ expected `bool`, found `&&bool`
|
help: consider removing the `&&`
|
LL - { true } && { true }
LL + { true } { true }
|
help: parentheses are required to parse this as an expression
|
LL | ({ true }) && { true }

View File

@ -676,6 +676,12 @@ error[E0308]: mismatched types
|
LL | if let Range { start: true, end } = t..&&false {}
| ^^^^^^^ expected `bool`, found `&&bool`
|
help: consider removing the `&&`
|
LL - if let Range { start: true, end } = t..&&false {}
LL + if let Range { start: true, end } = t..false {}
|
error[E0308]: mismatched types
--> $DIR/disallowed-positions.rs:83:8
@ -866,6 +872,12 @@ error[E0308]: mismatched types
|
LL | while let Range { start: true, end } = t..&&false {}
| ^^^^^^^ expected `bool`, found `&&bool`
|
help: consider removing the `&&`
|
LL - while let Range { start: true, end } = t..&&false {}
LL + while let Range { start: true, end } = t..false {}
|
error[E0308]: mismatched types
--> $DIR/disallowed-positions.rs:147:11

View File

@ -0,0 +1,26 @@
fn a(x: &&i32) -> i32 {
x
//~^ ERROR mismatched types
}
fn a2(x: &&&&&i32) -> i32 {
x
//~^ ERROR mismatched types
}
fn b(x: &i32) -> i32 {
&x
//~^ ERROR mismatched types
}
fn c(x: Box<i32>) -> i32 {
&x
//~^ ERROR mismatched types
}
fn d(x: std::sync::Mutex<&i32>) -> i32 {
x.lock().unwrap()
//~^ ERROR mismatched types
}
fn main() {}

View File

@ -0,0 +1,72 @@
error[E0308]: mismatched types
--> $DIR/deref-multi.rs:2:5
|
LL | fn a(x: &&i32) -> i32 {
| --- expected `i32` because of return type
LL | x
| ^ expected `i32`, found `&&i32`
|
help: consider dereferencing the borrow
|
LL | **x
| ++
error[E0308]: mismatched types
--> $DIR/deref-multi.rs:7:5
|
LL | fn a2(x: &&&&&i32) -> i32 {
| --- expected `i32` because of return type
LL | x
| ^ expected `i32`, found `&&&&&i32`
|
help: consider dereferencing the borrow
|
LL | *****x
| +++++
error[E0308]: mismatched types
--> $DIR/deref-multi.rs:12:5
|
LL | fn b(x: &i32) -> i32 {
| --- expected `i32` because of return type
LL | &x
| ^^ expected `i32`, found `&&i32`
|
help: consider removing the `&` and dereferencing the borrow instead
|
LL | *x
| ~
error[E0308]: mismatched types
--> $DIR/deref-multi.rs:17:5
|
LL | fn c(x: Box<i32>) -> i32 {
| --- expected `i32` because of return type
LL | &x
| ^^ expected `i32`, found `&Box<i32>`
|
= note: expected type `i32`
found reference `&Box<i32>`
help: consider removing the `&` and dereferencing the borrow instead
|
LL | *x
| ~
error[E0308]: mismatched types
--> $DIR/deref-multi.rs:22:5
|
LL | fn d(x: std::sync::Mutex<&i32>) -> i32 {
| --- expected `i32` because of return type
LL | x.lock().unwrap()
| ^^^^^^^^^^^^^^^^^ expected `i32`, found struct `MutexGuard`
|
= note: expected type `i32`
found struct `MutexGuard<'_, &i32>`
help: consider dereferencing the type
|
LL | **x.lock().unwrap()
| ++
error: aborting due to 5 previous errors
For more information about this error, try `rustc --explain E0308`.