From ce83be4af8462d8d6fadb2829c8ca7f4353fca9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 1 Jan 2023 19:35:53 -0800 Subject: [PATCH] Account for type params --- .../src/traits/error_reporting/suggestions.rs | 88 +++++++++++++------ tests/ui/kindck/kindck-copy.stderr | 12 ++- tests/ui/not-panic/not-panic-safe.rs | 4 +- tests/ui/not-panic/not-panic-safe.stderr | 15 ++-- 4 files changed, 81 insertions(+), 38 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 553144078ee..bcc82004804 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1363,19 +1363,70 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { // Remove all the hir desugaring contexts while maintaining the macro contexts. span.remove_mark(); } - let mut suggested = false; - - let mut expr_finder = super::FindExprBySpan { span, result: None }; + let mut expr_finder = super::FindExprBySpan::new(span); let Some(hir::Node::Expr(body)) = self.tcx.hir().find(obligation.cause.body_id) else { return false; }; expr_finder.visit_expr(&body); + let mut maybe_suggest = |suggested_ty, count, suggestions| { + // Remapping bound vars here + let trait_pred_and_suggested_ty = + trait_pred.map_bound(|trait_pred| (trait_pred, suggested_ty)); + + let new_obligation = self.mk_trait_obligation_with_new_self_ty( + obligation.param_env, + trait_pred_and_suggested_ty, + ); + + if self.predicate_may_hold(&new_obligation) { + let msg = if count == 1 { + "consider removing the leading `&`-reference".to_string() + } else { + format!("consider removing {count} leading `&`-references") + }; + + err.multipart_suggestion_verbose( + &msg, + suggestions, + Applicability::MachineApplicable, + ); + true + } else { + false + } + }; + + // Maybe suggest removal of borrows from types in type parameters, like in + // `src/test/ui/not-panic/not-panic-safe.rs`. let mut count = 0; let mut suggestions = vec![]; - let Some(mut expr) = expr_finder.result else { return false; }; // Skipping binder here, remapping below let mut suggested_ty = trait_pred.self_ty().skip_binder(); + if let Some(mut hir_ty) = expr_finder.ty_result { + while let hir::TyKind::Ref(_, mut_ty) = &hir_ty.kind { + count += 1; + let span = hir_ty.span.until(mut_ty.ty.span); + suggestions.push((span, String::new())); + let ty::Ref(_, inner_ty, _) = suggested_ty.kind() else { + break; + }; + suggested_ty = *inner_ty; + + hir_ty = mut_ty.ty; + + if maybe_suggest(suggested_ty, count, suggestions.clone()) { + return true; + } + } + } + + // Maybe suggest removal of borrows from expressions, like in `for i in &&&foo {}`. + let Some(mut expr) = expr_finder.result else { return false; }; + let mut count = 0; + let mut suggestions = vec![]; + // Skipping binder here, remapping below + let mut suggested_ty = trait_pred.self_ty().skip_binder(); 'outer: loop { while let hir::ExprKind::AddrOf(_, _, borrowed) = expr.kind { count += 1; @@ -1387,35 +1438,14 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { suggestions.push((span, String::new())); let ty::Ref(_, inner_ty, _) = suggested_ty.kind() else { - break; + break 'outer; }; suggested_ty = *inner_ty; expr = borrowed; - // Remapping bound vars here - let trait_pred_and_suggested_ty = - trait_pred.map_bound(|trait_pred| (trait_pred, suggested_ty)); - - let new_obligation = self.mk_trait_obligation_with_new_self_ty( - obligation.param_env, - trait_pred_and_suggested_ty, - ); - - if self.predicate_may_hold(&new_obligation) { - let msg = if count == 1 { - "consider removing the leading `&`-reference".to_string() - } else { - format!("consider removing {count} leading `&`-references") - }; - - err.multipart_suggestion_verbose( - &msg, - suggestions, - Applicability::MachineApplicable, - ); - suggested = true; - break 'outer; + if maybe_suggest(suggested_ty, count, suggestions.clone()) { + return true; } } if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind @@ -1431,7 +1461,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { break 'outer; } } - suggested + false } fn suggest_remove_await(&self, obligation: &PredicateObligation<'tcx>, err: &mut Diagnostic) { diff --git a/tests/ui/kindck/kindck-copy.stderr b/tests/ui/kindck/kindck-copy.stderr index 9af89159a8c..aee2aa98a60 100644 --- a/tests/ui/kindck/kindck-copy.stderr +++ b/tests/ui/kindck/kindck-copy.stderr @@ -4,12 +4,16 @@ error[E0277]: the trait bound `&'static mut isize: Copy` is not satisfied LL | assert_copy::<&'static mut isize>(); | ^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `&'static mut isize` | - = help: the trait `Copy` is implemented for `isize` note: required by a bound in `assert_copy` --> $DIR/kindck-copy.rs:5:18 | LL | fn assert_copy() { } | ^^^^ required by this bound in `assert_copy` +help: consider removing the leading `&`-reference + | +LL - assert_copy::<&'static mut isize>(); +LL + assert_copy::(); + | error[E0277]: the trait bound `&'a mut isize: Copy` is not satisfied --> $DIR/kindck-copy.rs:28:19 @@ -17,12 +21,16 @@ error[E0277]: the trait bound `&'a mut isize: Copy` is not satisfied LL | assert_copy::<&'a mut isize>(); | ^^^^^^^^^^^^^ the trait `Copy` is not implemented for `&'a mut isize` | - = help: the trait `Copy` is implemented for `isize` note: required by a bound in `assert_copy` --> $DIR/kindck-copy.rs:5:18 | LL | fn assert_copy() { } | ^^^^ required by this bound in `assert_copy` +help: consider removing the leading `&`-reference + | +LL - assert_copy::<&'a mut isize>(); +LL + assert_copy::(); + | error[E0277]: the trait bound `Box: Copy` is not satisfied --> $DIR/kindck-copy.rs:31:19 diff --git a/tests/ui/not-panic/not-panic-safe.rs b/tests/ui/not-panic/not-panic-safe.rs index 4165c5dc13a..1b3c6482ce9 100644 --- a/tests/ui/not-panic/not-panic-safe.rs +++ b/tests/ui/not-panic/not-panic-safe.rs @@ -5,6 +5,6 @@ use std::panic::UnwindSafe; fn assert() {} fn main() { - assert::<&mut i32>(); - //~^ ERROR the type `&mut i32` may not be safely transferred across an unwind boundary + assert::<&mut &mut &i32>(); + //~^ ERROR the type `&mut &mut &i32` may not be safely transferred across an unwind boundary } diff --git a/tests/ui/not-panic/not-panic-safe.stderr b/tests/ui/not-panic/not-panic-safe.stderr index 013c5ee7044..37a6aee3906 100644 --- a/tests/ui/not-panic/not-panic-safe.stderr +++ b/tests/ui/not-panic/not-panic-safe.stderr @@ -1,16 +1,21 @@ -error[E0277]: the type `&mut i32` may not be safely transferred across an unwind boundary +error[E0277]: the type `&mut &mut &i32` may not be safely transferred across an unwind boundary --> $DIR/not-panic-safe.rs:8:14 | -LL | assert::<&mut i32>(); - | ^^^^^^^^ `&mut i32` may not be safely transferred across an unwind boundary +LL | assert::<&mut &mut &i32>(); + | ^^^^^^^^^^^^^^ `&mut &mut &i32` may not be safely transferred across an unwind boundary | - = help: the trait `UnwindSafe` is not implemented for `&mut i32` - = note: `UnwindSafe` is implemented for `&i32`, but not for `&mut i32` + = help: the trait `UnwindSafe` is not implemented for `&mut &mut &i32` + = note: `UnwindSafe` is implemented for `&&mut &i32`, but not for `&mut &mut &i32` note: required by a bound in `assert` --> $DIR/not-panic-safe.rs:5:14 | LL | fn assert() {} | ^^^^^^^^^^ required by this bound in `assert` +help: consider removing 2 leading `&`-references + | +LL - assert::<&mut &mut &i32>(); +LL + assert::<&i32>(); + | error: aborting due to previous error