Skip to content
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

Jump to bottom

Suggest adding a reference to a trait assoc item #100769

Merged
merged 2 commits into from
Aug 20, 2022
Merged

Suggest adding a reference to a trait assoc item #100769

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64b3e4a
suggest adding a reference to a trait assoc item
TaKO8Ki Aug 19, 2022
a311b8a
use more descriptive names
TaKO8Ki Aug 20, 2022
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
186 changes: 101 additions & 85 deletions compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
obligation.cause.code()
{
&parent_code
} else if let ObligationCauseCode::ItemObligation(_) = obligation.cause.code() {
obligation.cause.code()
} else if let ExpnKind::Desugaring(DesugaringKind::ForLoop) =
span.ctxt().outer_expn_data().kind
{
Expand All @@ -906,102 +908,116 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let param_env = obligation.param_env;

// Try to apply the original trait binding obligation by borrowing.
let mut try_borrowing =
|old_pred: ty::PolyTraitPredicate<'tcx>, blacklist: &[DefId]| -> bool {
if blacklist.contains(&old_pred.def_id()) {
return false;
}
// We map bounds to `&T` and `&mut T`
let trait_pred_and_imm_ref = old_pred.map_bound(|trait_pred| {
(
trait_pred,
self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()),
)
});
let trait_pred_and_mut_ref = old_pred.map_bound(|trait_pred| {
(
trait_pred,
self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()),
)
});
let mut try_borrowing = |old_pred: ty::PolyTraitPredicate<'tcx>,
blacklist: &[DefId]|
-> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatting is a bit unorthodox here, but if rustfmt says so...

if blacklist.contains(&old_pred.def_id()) {
return false;
}
// We map bounds to `&T` and `&mut T`
let trait_pred_and_imm_ref = old_pred.map_bound(|trait_pred| {
(
trait_pred,
self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()),
)
});
let trait_pred_and_mut_ref = old_pred.map_bound(|trait_pred| {
(
trait_pred,
self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, trait_pred.self_ty()),
)
});

let mk_result = |trait_pred_and_new_ty| {
let obligation =
self.mk_trait_obligation_with_new_self_ty(param_env, trait_pred_and_new_ty);
self.predicate_must_hold_modulo_regions(&obligation)
let mk_result = |trait_pred_and_new_ty| {
let obligation =
self.mk_trait_obligation_with_new_self_ty(param_env, trait_pred_and_new_ty);
self.predicate_must_hold_modulo_regions(&obligation)
};
let imm_result = mk_result(trait_pred_and_imm_ref);
let mut_result = mk_result(trait_pred_and_mut_ref);
TaKO8Ki marked this conversation as resolved.
Show resolved Hide resolved

let ref_inner_ty_result =
TaKO8Ki marked this conversation as resolved.
Show resolved Hide resolved
if let ObligationCauseCode::ItemObligation(_) = obligation.cause.code()
&& let ty::Ref(_, ty, mutability) = old_pred.self_ty().skip_binder().kind()
{
Some((mk_result(old_pred.map_bound(|trait_pred| (trait_pred, *ty))), mutability))
} else {
None
};
let imm_result = mk_result(trait_pred_and_imm_ref);
let mut_result = mk_result(trait_pred_and_mut_ref);

if imm_result || mut_result {
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
// 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", old_pred);
if has_custom_message {
err.note(&msg);
} else {
err.message =
vec![(rustc_errors::DiagnosticMessage::Str(msg), Style::NoStyle)];
}
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;
}
err.span_label(
span,
&format!(
"expected an implementor of trait `{}`",
old_pred.print_modifiers_and_trait_path(),
),
);

// This if is to prevent a special edge-case
if matches!(
span.ctxt().outer_expn_data().kind,
ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop)
) {
// We don't want a borrowing suggestion on the fields in structs,
// ```
// struct Foo {
// the_foos: Vec<Foo>
// }
// ```
if imm_result || mut_result || ref_inner_ty_result.map_or(false, |(result, _)| result) {
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
// 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", old_pred);
if has_custom_message {
err.note(&msg);
} else {
err.message =
vec![(rustc_errors::DiagnosticMessage::Str(msg), Style::NoStyle)];
}
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;
}
err.span_label(
span,
&format!(
"expected an implementor of trait `{}`",
old_pred.print_modifiers_and_trait_path(),
),
);

if imm_result && mut_result {
err.span_suggestions(
span.shrink_to_lo(),
"consider borrowing here",
["&".to_string(), "&mut ".to_string()].into_iter(),
Applicability::MaybeIncorrect,
);
} else {
err.span_suggestion_verbose(
span.shrink_to_lo(),
&format!(
"consider{} borrowing here",
if mut_result { " mutably" } else { "" }
),
format!("&{}", if mut_result { "mut " } else { "" }),
Applicability::MaybeIncorrect,
);
}
// This if is to prevent a special edge-case
if matches!(
span.ctxt().outer_expn_data().kind,
ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop)
) {
// We don't want a borrowing suggestion on the fields in structs,
// ```
// struct Foo {
// the_foos: Vec<Foo>
// }
// ```

if imm_result && mut_result {
err.span_suggestions(
span.shrink_to_lo(),
"consider borrowing here",
["&".to_string(), "&mut ".to_string()].into_iter(),
Applicability::MaybeIncorrect,
);
} else {
let is_mut = mut_result
|| ref_inner_ty_result.map_or(false, |(_, mutabl)| {
matches!(mutabl, hir::Mutability::Mut)
});
err.span_suggestion_verbose(
span.shrink_to_lo(),
&format!(
"consider{} borrowing here",
if is_mut { " mutably" } else { "" }
),
format!("&{}", if is_mut { "mut " } else { "" }),
Applicability::MaybeIncorrect,
);
}
return true;
}
return true;
}
return false;
};
}
return false;
};

if let ObligationCauseCode::ImplDerivedObligation(cause) = &*code {
try_borrowing(cause.derived.parent_trait_pred, &[])
} else if let ObligationCauseCode::BindingObligation(_, _)
| ObligationCauseCode::ItemObligation(_) = code
| ObligationCauseCode::ItemObligation(..) = code
{
try_borrowing(poly_trait_pred, &never_suggest_borrow)
} else {
Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// run-rustfix
#![allow(unused_variables)]

fn foo(foo: &mut usize) {
todo!()
}

fn bar(bar: &usize) {
todo!()
}

fn main() {
foo(&mut Default::default()); //~ the trait bound `&mut usize: Default` is not satisfied
bar(&Default::default()); //~ the trait bound `&usize: Default` is not satisfied
}
15 changes: 15 additions & 0 deletions src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// run-rustfix
#![allow(unused_variables)]

fn foo(foo: &mut usize) {
todo!()
}

fn bar(bar: &usize) {
todo!()
}

fn main() {
foo(Default::default()); //~ the trait bound `&mut usize: Default` is not satisfied
bar(Default::default()); //~ the trait bound `&usize: Default` is not satisfied
}
25 changes: 25 additions & 0 deletions src/test/ui/suggestions/suggest-adding-reference-to-trait-assoc-item.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error[E0277]: the trait bound `&mut usize: Default` is not satisfied
--> $DIR/suggest-adding-reference-to-trait-assoc-item.rs:13:9
|
LL | foo(Default::default());
| ^^^^^^^^^^^^^^^^ expected an implementor of trait `Default`
|
help: consider mutably borrowing here
|
LL | foo(&mut Default::default());
| ++++

error[E0277]: the trait bound `&usize: Default` is not satisfied
--> $DIR/suggest-adding-reference-to-trait-assoc-item.rs:14:9
|
LL | bar(Default::default());
| ^^^^^^^^^^^^^^^^ expected an implementor of trait `Default`
|
help: consider borrowing here
|
LL | bar(&Default::default());
| +

error: aborting due to 2 previous errors

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