Skip to content

Commit a85c1bb

Browse files
tgross35tgross35
authored
Rollup merge of rust-lang#143742 - estebank:borrow-suggestion, r=compiler-errors
Rework borrowing suggestions to use `Expr` instead of just `Span` In the suggestion machinery for borrowing expressions and types, always use the available obligation `Span` to find the appropriate `Expr` to perform appropriateness checks no the `ExprKind` instead of on the textual snippet corresponding to the `Span`. (We were already doing this, but only for a subset of cases.) This now better handles situations where parentheses and `<>` are needed for correct syntax (`&(foo + bar)`, `(&foo).bar()`, `<&Foo>::bar()`, etc.). Unify the logic for the case where `&` *and* `&mut` are appropriate with the logic for only one of those cases. (Instead of having two branches for emitting the suggestion, we now have a single one, using `Diag::multipart_suggestions` always.) Handle the case when `S::foo()` should have been `<&S>::foo()` (instead of suggesting the prior `&S::foo()`. Fix rust-lang#143393. Make `Diag::multipart_suggestions` always verbose. CC rust-lang#141973.
2 parents 3ebdb8b + d692444 commit a85c1bb

19 files changed

+316
-179
lines changed

‎compiler/rustc_errors/src/diagnostic.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1165,7 +1165,7 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
11651165
self.push_suggestion(CodeSuggestion {
11661166
substitutions,
11671167
msg: self.subdiagnostic_message_to_diagnostic_message(msg),
1168-
style: SuggestionStyle::ShowCode,
1168+
style: SuggestionStyle::ShowAlways,
11691169
applicability,
11701170
});
11711171
self

‎compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs‎

Lines changed: 125 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,7 +1321,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
13211321
let imm_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_imm_ref);
13221322
let mut_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_mut_ref);
13231323

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

1336-
if imm_ref_self_ty_satisfies_pred
1337-
|| mut_ref_self_ty_satisfies_pred
1338-
|| ref_inner_ty_satisfies_pred
1339-
{
1340-
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
1341-
// We don't want a borrowing suggestion on the fields in structs,
1342-
// ```
1343-
// struct Foo {
1344-
// the_foos: Vec<Foo>
1345-
// }
1346-
// ```
1347-
if !matches!(
1348-
span.ctxt().outer_expn_data().kind,
1349-
ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop)
1350-
) {
1351-
return false;
1352-
}
1353-
if snippet.starts_with('&') {
1354-
// This is already a literal borrow and the obligation is failing
1355-
// somewhere else in the obligation chain. Do not suggest non-sense.
1356-
return false;
1357-
}
1358-
// We have a very specific type of error, where just borrowing this argument
1359-
// might solve the problem. In cases like this, the important part is the
1360-
// original type obligation, not the last one that failed, which is arbitrary.
1361-
// Because of this, we modify the error to refer to the original obligation and
1362-
// return early in the caller.
1363-
1364-
let msg = format!(
1365-
"the trait bound `{}` is not satisfied",
1366-
self.tcx.short_string(old_pred, err.long_ty_path()),
1367-
);
1368-
let self_ty_str =
1369-
self.tcx.short_string(old_pred.self_ty().skip_binder(), err.long_ty_path());
1370-
if has_custom_message {
1371-
err.note(msg);
1372-
} else {
1373-
err.messages = vec![(rustc_errors::DiagMessage::from(msg), Style::NoStyle)];
1374-
}
1375-
err.span_label(
1376-
span,
1377-
format!(
1378-
"the trait `{}` is not implemented for `{self_ty_str}`",
1379-
old_pred.print_modifiers_and_trait_path()
1380-
),
1381-
);
1336+
let is_immut = imm_ref_self_ty_satisfies_pred
1337+
|| (ref_inner_ty_satisfies_pred && !ref_inner_ty_is_mut);
1338+
let is_mut = mut_ref_self_ty_satisfies_pred || ref_inner_ty_is_mut;
1339+
if !is_immut && !is_mut {
1340+
return false;
1341+
}
1342+
let Ok(_snippet) = self.tcx.sess.source_map().span_to_snippet(span) else {
1343+
return false;
1344+
};
1345+
// We don't want a borrowing suggestion on the fields in structs
1346+
// ```
1347+
// #[derive(Clone)]
1348+
// struct Foo {
1349+
// the_foos: Vec<Foo>
1350+
// }
1351+
// ```
1352+
if !matches!(
1353+
span.ctxt().outer_expn_data().kind,
1354+
ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop)
1355+
) {
1356+
return false;
1357+
}
1358+
// We have a very specific type of error, where just borrowing this argument
1359+
// might solve the problem. In cases like this, the important part is the
1360+
// original type obligation, not the last one that failed, which is arbitrary.
1361+
// Because of this, we modify the error to refer to the original obligation and
1362+
// return early in the caller.
13821363

1383-
if imm_ref_self_ty_satisfies_pred && mut_ref_self_ty_satisfies_pred {
1384-
err.span_suggestions(
1385-
span.shrink_to_lo(),
1386-
"consider borrowing here",
1387-
["&".to_string(), "&mut ".to_string()],
1388-
Applicability::MaybeIncorrect,
1389-
);
1390-
} else {
1391-
let is_mut = mut_ref_self_ty_satisfies_pred || ref_inner_ty_mut;
1392-
let sugg_prefix = format!("&{}", if is_mut { "mut " } else { "" });
1393-
let sugg_msg = format!(
1394-
"consider{} borrowing here",
1395-
if is_mut { " mutably" } else { "" }
1396-
);
1364+
let mut label = || {
1365+
let msg = format!(
1366+
"the trait bound `{}` is not satisfied",
1367+
self.tcx.short_string(old_pred, err.long_ty_path()),
1368+
);
1369+
let self_ty_str =
1370+
self.tcx.short_string(old_pred.self_ty().skip_binder(), err.long_ty_path());
1371+
if has_custom_message {
1372+
err.note(msg);
1373+
} else {
1374+
err.messages = vec![(rustc_errors::DiagMessage::from(msg), Style::NoStyle)];
1375+
}
1376+
err.span_label(
1377+
span,
1378+
format!(
1379+
"the trait `{}` is not implemented for `{self_ty_str}`",
1380+
old_pred.print_modifiers_and_trait_path()
1381+
),
1382+
);
1383+
};
13971384

1398-
// Issue #109436, we need to add parentheses properly for method calls
1399-
// for example, `foo.into()` should be `(&foo).into()`
1400-
if let Some(_) =
1401-
self.tcx.sess.source_map().span_look_ahead(span, ".", Some(50))
1402-
{
1403-
err.multipart_suggestion_verbose(
1404-
sugg_msg,
1405-
vec![
1406-
(span.shrink_to_lo(), format!("({sugg_prefix}")),
1407-
(span.shrink_to_hi(), ")".to_string()),
1408-
],
1409-
Applicability::MaybeIncorrect,
1410-
);
1411-
return true;
1412-
}
1385+
let mut sugg_prefixes = vec![];
1386+
if is_immut {
1387+
sugg_prefixes.push("&");
1388+
}
1389+
if is_mut {
1390+
sugg_prefixes.push("&mut ");
1391+
}
1392+
let sugg_msg = format!(
1393+
"consider{} borrowing here",
1394+
if is_mut && !is_immut { " mutably" } else { "" },
1395+
);
14131396

1414-
// Issue #104961, we need to add parentheses properly for compound expressions
1415-
// for example, `x.starts_with("hi".to_string() + "you")`
1416-
// should be `x.starts_with(&("hi".to_string() + "you"))`
1417-
let Some(body) = self.tcx.hir_maybe_body_owned_by(obligation.cause.body_id)
1418-
else {
1419-
return false;
1420-
};
1421-
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
1422-
expr_finder.visit_expr(body.value);
1423-
let Some(expr) = expr_finder.result else {
1424-
return false;
1425-
};
1426-
let needs_parens = expr_needs_parens(expr);
1397+
// Issue #104961, we need to add parentheses properly for compound expressions
1398+
// for example, `x.starts_with("hi".to_string() + "you")`
1399+
// should be `x.starts_with(&("hi".to_string() + "you"))`
1400+
let Some(body) = self.tcx.hir_maybe_body_owned_by(obligation.cause.body_id) else {
1401+
return false;
1402+
};
1403+
let mut expr_finder = FindExprBySpan::new(span, self.tcx);
1404+
expr_finder.visit_expr(body.value);
14271405

1428-
let span = if needs_parens { span } else { span.shrink_to_lo() };
1429-
let suggestions = if !needs_parens {
1430-
vec![(span.shrink_to_lo(), sugg_prefix)]
1431-
} else {
1406+
if let Some(ty) = expr_finder.ty_result {
1407+
if let hir::Node::Expr(expr) = self.tcx.parent_hir_node(ty.hir_id)
1408+
&& let hir::ExprKind::Path(hir::QPath::TypeRelative(_, _)) = expr.kind
1409+
&& ty.span == span
1410+
{
1411+
// We've encountered something like `str::from("")`, where the intended code
1412+
// was likely `<&str>::from("")`. #143393.
1413+
label();
1414+
err.multipart_suggestions(
1415+
sugg_msg,
1416+
sugg_prefixes.into_iter().map(|sugg_prefix| {
14321417
vec![
1433-
(span.shrink_to_lo(), format!("{sugg_prefix}(")),
1434-
(span.shrink_to_hi(), ")".to_string()),
1418+
(span.shrink_to_lo(), format!("<{sugg_prefix}")),
1419+
(span.shrink_to_hi(), ">".to_string()),
14351420
]
1436-
};
1437-
err.multipart_suggestion_verbose(
1438-
sugg_msg,
1439-
suggestions,
1440-
Applicability::MaybeIncorrect,
1441-
);
1442-
}
1421+
}),
1422+
Applicability::MaybeIncorrect,
1423+
);
14431424
return true;
14441425
}
1426+
return false;
14451427
}
1446-
return false;
1428+
let Some(expr) = expr_finder.result else {
1429+
return false;
1430+
};
1431+
if let hir::ExprKind::AddrOf(_, _, _) = expr.kind {
1432+
return false;
1433+
}
1434+
let needs_parens_post = expr_needs_parens(expr);
1435+
let needs_parens_pre = match self.tcx.parent_hir_node(expr.hir_id) {
1436+
Node::Expr(e)
1437+
if let hir::ExprKind::MethodCall(_, base, _, _) = e.kind
1438+
&& base.hir_id == expr.hir_id =>
1439+
{
1440+
true
1441+
}
1442+
_ => false,
1443+
};
1444+
1445+
label();
1446+
let suggestions = sugg_prefixes.into_iter().map(|sugg_prefix| {
1447+
match (needs_parens_pre, needs_parens_post) {
1448+
(false, false) => vec![(span.shrink_to_lo(), sugg_prefix.to_string())],
1449+
// We have something like `foo.bar()`, where we want to bororw foo, so we need
1450+
// to suggest `(&mut foo).bar()`.
1451+
(false, true) => vec![
1452+
(span.shrink_to_lo(), format!("{sugg_prefix}(")),
1453+
(span.shrink_to_hi(), ")".to_string()),
1454+
],
1455+
// Issue #109436, we need to add parentheses properly for method calls
1456+
// for example, `foo.into()` should be `(&foo).into()`
1457+
(true, false) => vec![
1458+
(span.shrink_to_lo(), format!("({sugg_prefix}")),
1459+
(span.shrink_to_hi(), ")".to_string()),
1460+
],
1461+
(true, true) => vec![
1462+
(span.shrink_to_lo(), format!("({sugg_prefix}(")),
1463+
(span.shrink_to_hi(), "))".to_string()),
1464+
],
1465+
}
1466+
});
1467+
err.multipart_suggestions(sugg_msg, suggestions, Applicability::MaybeIncorrect);
1468+
return true;
14471469
};
14481470

14491471
if let ObligationCauseCode::ImplDerived(cause) = &*code {

‎tests/ui/consts/const-size_of_val-align_of_val-extern-type.stderr‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,37 @@ error[E0277]: the size for values of type `Opaque` cannot be known
22
--> $DIR/const-size_of_val-align_of_val-extern-type.rs:10:43
33
|
44
LL | const _SIZE: usize = unsafe { size_of_val(&4 as *const i32 as *const Opaque) };
5-
| ----------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a known size
5+
| ----------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `MetaSized` is not implemented for `Opaque`
66
| |
77
| required by a bound introduced by this call
88
|
9-
= help: the trait `MetaSized` is not implemented for `Opaque`
9+
= note: the trait bound `Opaque: MetaSized` is not satisfied
1010
note: required by a bound in `std::intrinsics::size_of_val`
1111
--> $SRC_DIR/core/src/intrinsics/mod.rs:LL:COL
12+
help: consider borrowing here
13+
|
14+
LL | const _SIZE: usize = unsafe { size_of_val(&(&4 as *const i32 as *const Opaque)) };
15+
| ++ +
16+
LL | const _SIZE: usize = unsafe { size_of_val(&mut (&4 as *const i32 as *const Opaque)) };
17+
| ++++++ +
1218

1319
error[E0277]: the size for values of type `Opaque` cannot be known
1420
--> $DIR/const-size_of_val-align_of_val-extern-type.rs:12:45
1521
|
1622
LL | const _ALIGN: usize = unsafe { align_of_val(&4 as *const i32 as *const Opaque) };
17-
| ------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a known size
23+
| ------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `MetaSized` is not implemented for `Opaque`
1824
| |
1925
| required by a bound introduced by this call
2026
|
21-
= help: the trait `MetaSized` is not implemented for `Opaque`
27+
= note: the trait bound `Opaque: MetaSized` is not satisfied
2228
note: required by a bound in `std::intrinsics::align_of_val`
2329
--> $SRC_DIR/core/src/intrinsics/mod.rs:LL:COL
30+
help: consider borrowing here
31+
|
32+
LL | const _ALIGN: usize = unsafe { align_of_val(&(&4 as *const i32 as *const Opaque)) };
33+
| ++ +
34+
LL | const _ALIGN: usize = unsafe { align_of_val(&mut (&4 as *const i32 as *const Opaque)) };
35+
| ++++++ +
2436

2537
error: aborting due to 2 previous errors
2638

‎tests/ui/extern/unsized-extern-derefmove.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ note: required by a bound in `Box::<T>::from_raw`
2121
--> $SRC_DIR/alloc/src/boxed.rs:LL:COL
2222
help: consider borrowing here
2323
|
24-
LL | Box::from_raw(&0 as *mut _)
25-
| +
26-
LL | Box::from_raw(&mut 0 as *mut _)
27-
| ++++
24+
LL | Box::from_raw(&(0 as *mut _))
25+
| ++ +
26+
LL | Box::from_raw(&mut (0 as *mut _))
27+
| ++++++ +
2828

2929
error[E0277]: the size for values of type `Device` cannot be known
3030
--> $DIR/unsized-extern-derefmove.rs:11:5

‎tests/ui/impl-trait/in-trait/default-body-type-err-2.stderr‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ error[E0308]: mismatched types
44
LL | async fn woopsie_async(&self) -> String {
55
| ------ expected `String` because of return type
66
LL | 42
7-
| ^^- help: try using a conversion method: `.to_string()`
8-
| |
9-
| expected `String`, found integer
7+
| ^^ expected `String`, found integer
8+
|
9+
help: try using a conversion method
10+
|
11+
LL | 42.to_string()
12+
| ++++++++++++
1013

1114
error: aborting due to 1 previous error
1215

‎tests/ui/inference/deref-suggestion.stderr‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,19 @@ error[E0308]: mismatched types
22
--> $DIR/deref-suggestion.rs:8:9
33
|
44
LL | foo(s);
5-
| --- ^- help: try using a conversion method: `.to_string()`
6-
| | |
7-
| | expected `String`, found `&String`
5+
| --- ^ expected `String`, found `&String`
6+
| |
87
| arguments to this function are incorrect
98
|
109
note: function defined here
1110
--> $DIR/deref-suggestion.rs:5:4
1211
|
1312
LL | fn foo(_: String) {}
1413
| ^^^ ---------
14+
help: try using a conversion method
15+
|
16+
LL | foo(s.to_string());
17+
| ++++++++++++
1518

1619
error[E0308]: mismatched types
1720
--> $DIR/deref-suggestion.rs:14:10

‎tests/ui/repeat-expr/typo-in-repeat-expr-issue-80173.stderr‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,12 @@ error[E0308]: mismatched types
3838
--> $DIR/typo-in-repeat-expr-issue-80173.rs:32:29
3939
|
4040
LL | let e = [String::new(), 10];
41-
| ^^- help: try using a conversion method: `.to_string()`
42-
| |
43-
| expected `String`, found integer
41+
| ^^ expected `String`, found integer
42+
|
43+
help: try using a conversion method
44+
|
45+
LL | let e = [String::new(), 10.to_string()];
46+
| ++++++++++++
4447

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

0 commit comments

Comments
 (0)