Skip to content

Rework borrowing suggestions to use Expr instead of just Span #143742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 11, 2025
Merged

Rework borrowing suggestions to use Expr instead of just Span #143742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7dfc3e9
Rework borrowing suggestions to use `Expr` instead of just `Span`
estebank Jul 10, 2025
0674eca
Make `Diag::multipart_suggestions` always verbose
estebank Jul 10, 2025
d692444
Update test
estebank Jul 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/diagnostic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
self.push_suggestion(CodeSuggestion {
substitutions,
msg: self.subdiagnostic_message_to_diagnostic_message(msg),
style: SuggestionStyle::ShowCode,
style: SuggestionStyle::ShowAlways,
applicability,
});
self
Expand Down
228 changes: 125 additions & 103 deletions compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1321,7 +1321,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
let imm_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_imm_ref);
let mut_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_mut_ref);

let (ref_inner_ty_satisfies_pred, ref_inner_ty_mut) =
let (ref_inner_ty_satisfies_pred, ref_inner_ty_is_mut) =
if let ObligationCauseCode::WhereClauseInExpr(..) = obligation.cause.code()
&& let ty::Ref(_, ty, mutability) = old_pred.self_ty().skip_binder().kind()
{
Expand All @@ -1333,117 +1333,139 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
(false, false)
};

if imm_ref_self_ty_satisfies_pred
|| mut_ref_self_ty_satisfies_pred
|| ref_inner_ty_satisfies_pred
{
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
// We don't want a borrowing suggestion on the fields in structs,
// ```
// struct Foo {
// the_foos: Vec<Foo>
// }
// ```
if !matches!(
span.ctxt().outer_expn_data().kind,
ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop)
) {
return false;
}
if snippet.starts_with('&') {
// This is already a literal borrow and the obligation is failing
// somewhere else in the obligation chain. Do not suggest non-sense.
return false;
}
// We have a very specific type of error, where just borrowing this argument
// might solve the problem. In cases like this, the important part is the
// original type obligation, not the last one that failed, which is arbitrary.
// Because of this, we modify the error to refer to the original obligation and
// return early in the caller.

let msg = format!(
"the trait bound `{}` is not satisfied",
self.tcx.short_string(old_pred, err.long_ty_path()),
);
let self_ty_str =
self.tcx.short_string(old_pred.self_ty().skip_binder(), err.long_ty_path());
if has_custom_message {
err.note(msg);
} else {
err.messages = vec![(rustc_errors::DiagMessage::from(msg), Style::NoStyle)];
}
err.span_label(
span,
format!(
"the trait `{}` is not implemented for `{self_ty_str}`",
old_pred.print_modifiers_and_trait_path()
),
);
let is_immut = imm_ref_self_ty_satisfies_pred
|| (ref_inner_ty_satisfies_pred && !ref_inner_ty_is_mut);
let is_mut = mut_ref_self_ty_satisfies_pred || ref_inner_ty_is_mut;
if !is_immut && !is_mut {
return false;
}
let Ok(_snippet) = self.tcx.sess.source_map().span_to_snippet(span) else {
return false;
};
// We don't want a borrowing suggestion on the fields in structs
// ```
// #[derive(Clone)]
// struct Foo {
// the_foos: Vec<Foo>
// }
// ```
if !matches!(
span.ctxt().outer_expn_data().kind,
ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop)
) {
return false;
}
// We have a very specific type of error, where just borrowing this argument
// might solve the problem. In cases like this, the important part is the
// original type obligation, not the last one that failed, which is arbitrary.
// Because of this, we modify the error to refer to the original obligation and
// return early in the caller.

if imm_ref_self_ty_satisfies_pred && mut_ref_self_ty_satisfies_pred {
err.span_suggestions(
span.shrink_to_lo(),
"consider borrowing here",
["&".to_string(), "&mut ".to_string()],
Applicability::MaybeIncorrect,
);
} else {
let is_mut = mut_ref_self_ty_satisfies_pred || ref_inner_ty_mut;
let sugg_prefix = format!("&{}", if is_mut { "mut " } else { "" });
let sugg_msg = format!(
"consider{} borrowing here",
if is_mut { " mutably" } else { "" }
);
let mut label = || {
let msg = format!(
"the trait bound `{}` is not satisfied",
self.tcx.short_string(old_pred, err.long_ty_path()),
);
let self_ty_str =
self.tcx.short_string(old_pred.self_ty().skip_binder(), err.long_ty_path());
if has_custom_message {
err.note(msg);
} else {
err.messages = vec![(rustc_errors::DiagMessage::from(msg), Style::NoStyle)];
}
err.span_label(
span,
format!(
"the trait `{}` is not implemented for `{self_ty_str}`",
old_pred.print_modifiers_and_trait_path()
),
);
};

// Issue #109436, we need to add parentheses properly for method calls
// for example, `foo.into()` should be `(&foo).into()`
if let Some(_) =
self.tcx.sess.source_map().span_look_ahead(span, ".", Some(50))
{
err.multipart_suggestion_verbose(
sugg_msg,
vec![
(span.shrink_to_lo(), format!("({sugg_prefix}")),
(span.shrink_to_hi(), ")".to_string()),
],
Applicability::MaybeIncorrect,
);
return true;
}
let mut sugg_prefixes = vec![];
if is_immut {
sugg_prefixes.push("&");
}
if is_mut {
sugg_prefixes.push("&mut ");
}
let sugg_msg = format!(
"consider{} borrowing here",
if is_mut && !is_immut { " mutably" } else { "" },
);

// Issue #104961, we need to add parentheses properly for compound expressions
// for example, `x.starts_with("hi".to_string() + "you")`
// should be `x.starts_with(&("hi".to_string() + "you"))`
let Some(body) = self.tcx.hir_maybe_body_owned_by(obligation.cause.body_id)
else {
return false;
};
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
expr_finder.visit_expr(body.value);
let Some(expr) = expr_finder.result else {
return false;
};
let needs_parens = expr_needs_parens(expr);
// Issue #104961, we need to add parentheses properly for compound expressions
// for example, `x.starts_with("hi".to_string() + "you")`
// should be `x.starts_with(&("hi".to_string() + "you"))`
let Some(body) = self.tcx.hir_maybe_body_owned_by(obligation.cause.body_id) else {
return false;
};
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
expr_finder.visit_expr(body.value);

let span = if needs_parens { span } else { span.shrink_to_lo() };
let suggestions = if !needs_parens {
vec![(span.shrink_to_lo(), sugg_prefix)]
} else {
if let Some(ty) = expr_finder.ty_result {
if let hir::Node::Expr(expr) = self.tcx.parent_hir_node(ty.hir_id)
&& let hir::ExprKind::Path(hir::QPath::TypeRelative(_, _)) = expr.kind
&& ty.span == span
{
// We've encountered something like `str::from("")`, where the intended code
// was likely `<&str>::from("")`. #143393.
label();
err.multipart_suggestions(
sugg_msg,
sugg_prefixes.into_iter().map(|sugg_prefix| {
vec![
(span.shrink_to_lo(), format!("{sugg_prefix}(")),
(span.shrink_to_hi(), ")".to_string()),
(span.shrink_to_lo(), format!("<{sugg_prefix}")),
(span.shrink_to_hi(), ">".to_string()),
]
};
err.multipart_suggestion_verbose(
sugg_msg,
suggestions,
Applicability::MaybeIncorrect,
);
}
}),
Applicability::MaybeIncorrect,
);
return true;
}
return false;
}
return false;
let Some(expr) = expr_finder.result else {
return false;
};
if let hir::ExprKind::AddrOf(_, _, _) = expr.kind {
return false;
}
let needs_parens_post = expr_needs_parens(expr);
let needs_parens_pre = match self.tcx.parent_hir_node(expr.hir_id) {
Node::Expr(e)
if let hir::ExprKind::MethodCall(_, base, _, _) = e.kind
&& base.hir_id == expr.hir_id =>
{
true
}
_ => false,
};

label();
let suggestions = sugg_prefixes.into_iter().map(|sugg_prefix| {
match (needs_parens_pre, needs_parens_post) {
(false, false) => vec![(span.shrink_to_lo(), sugg_prefix.to_string())],
// We have something like `foo.bar()`, where we want to bororw foo, so we need
// to suggest `(&mut foo).bar()`.
(false, true) => vec![
(span.shrink_to_lo(), format!("{sugg_prefix}(")),
(span.shrink_to_hi(), ")".to_string()),
],
// Issue #109436, we need to add parentheses properly for method calls
// for example, `foo.into()` should be `(&foo).into()`
(true, false) => vec![
(span.shrink_to_lo(), format!("({sugg_prefix}")),
(span.shrink_to_hi(), ")".to_string()),
],
(true, true) => vec![
(span.shrink_to_lo(), format!("({sugg_prefix}(")),
(span.shrink_to_hi(), "))".to_string()),
],
}
});
err.multipart_suggestions(sugg_msg, suggestions, Applicability::MaybeIncorrect);
return true;
};

if let ObligationCauseCode::ImplDerived(cause) = &*code {
Expand Down
20 changes: 16 additions & 4 deletions tests/ui/consts/const-size_of_val-align_of_val-extern-type.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,37 @@ error[E0277]: the size for values of type `Opaque` cannot be known
--> $DIR/const-size_of_val-align_of_val-extern-type.rs:10:43
|
LL | const _SIZE: usize = unsafe { size_of_val(&4 as *const i32 as *const Opaque) };
| ----------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a known size
| ----------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `MetaSized` is not implemented for `Opaque`
| |
| required by a bound introduced by this call
|
= help: the trait `MetaSized` is not implemented for `Opaque`
= note: the trait bound `Opaque: MetaSized` is not satisfied
note: required by a bound in `std::intrinsics::size_of_val`
--> $SRC_DIR/core/src/intrinsics/mod.rs:LL:COL
help: consider borrowing here
|
LL | const _SIZE: usize = unsafe { size_of_val(&(&4 as *const i32 as *const Opaque)) };
| ++ +
LL | const _SIZE: usize = unsafe { size_of_val(&mut (&4 as *const i32 as *const Opaque)) };
| ++++++ +

error[E0277]: the size for values of type `Opaque` cannot be known
--> $DIR/const-size_of_val-align_of_val-extern-type.rs:12:45
|
LL | const _ALIGN: usize = unsafe { align_of_val(&4 as *const i32 as *const Opaque) };
| ------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a known size
| ------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `MetaSized` is not implemented for `Opaque`
| |
| required by a bound introduced by this call
|
= help: the trait `MetaSized` is not implemented for `Opaque`
= note: the trait bound `Opaque: MetaSized` is not satisfied
note: required by a bound in `std::intrinsics::align_of_val`
--> $SRC_DIR/core/src/intrinsics/mod.rs:LL:COL
help: consider borrowing here
|
LL | const _ALIGN: usize = unsafe { align_of_val(&(&4 as *const i32 as *const Opaque)) };
| ++ +
LL | const _ALIGN: usize = unsafe { align_of_val(&mut (&4 as *const i32 as *const Opaque)) };
| ++++++ +

error: aborting due to 2 previous errors

Expand Down
8 changes: 4 additions & 4 deletions tests/ui/extern/unsized-extern-derefmove.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ note: required by a bound in `Box::<T>::from_raw`
--> $SRC_DIR/alloc/src/boxed.rs:LL:COL
help: consider borrowing here
|
LL | Box::from_raw(&0 as *mut _)
| +
LL | Box::from_raw(&mut 0 as *mut _)
| ++++
LL | Box::from_raw(&(0 as *mut _))
| ++ +
LL | Box::from_raw(&mut (0 as *mut _))
| ++++++ +

error[E0277]: the size for values of type `Device` cannot be known
--> $DIR/unsized-extern-derefmove.rs:11:5
Expand Down
9 changes: 6 additions & 3 deletions tests/ui/impl-trait/in-trait/default-body-type-err-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ error[E0308]: mismatched types
LL | async fn woopsie_async(&self) -> String {
| ------ expected `String` because of return type
LL | 42
| ^^- help: try using a conversion method: `.to_string()`
| |
| expected `String`, found integer
| ^^ expected `String`, found integer
|
help: try using a conversion method
|
LL | 42.to_string()
| ++++++++++++

error: aborting due to 1 previous error

Expand Down
9 changes: 6 additions & 3 deletions tests/ui/inference/deref-suggestion.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@ error[E0308]: mismatched types
--> $DIR/deref-suggestion.rs:8:9
|
LL | foo(s);
| --- ^- help: try using a conversion method: `.to_string()`
| | |
| | expected `String`, found `&String`
| --- ^ expected `String`, found `&String`
| |
| arguments to this function are incorrect
|
note: function defined here
--> $DIR/deref-suggestion.rs:5:4
|
LL | fn foo(_: String) {}
| ^^^ ---------
help: try using a conversion method
|
LL | foo(s.to_string());
| ++++++++++++

error[E0308]: mismatched types
--> $DIR/deref-suggestion.rs:14:10
Expand Down
9 changes: 6 additions & 3 deletions tests/ui/repeat-expr/typo-in-repeat-expr-issue-80173.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,12 @@ error[E0308]: mismatched types
--> $DIR/typo-in-repeat-expr-issue-80173.rs:32:29
|
LL | let e = [String::new(), 10];
| ^^- help: try using a conversion method: `.to_string()`
| |
| expected `String`, found integer
| ^^ expected `String`, found integer
|
help: try using a conversion method
|
LL | let e = [String::new(), 10.to_string()];
| ++++++++++++

error[E0308]: mismatched types
--> $DIR/typo-in-repeat-expr-issue-80173.rs:36:19
Expand Down
Loading
Loading