From 4ca876b7a43c52a51cfcaf865a29a3ff27059b26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 15 Mar 2024 18:08:12 +0000 Subject: [PATCH] Better account for `FnOnce` in move errors ``` error[E0382]: use of moved value: `blk` --> $DIR/once-cant-call-twice-on-heap.rs:8:5 | LL | fn foo(blk: F) { | --- move occurs because `blk` has type `F`, which does not implement the `Copy` trait LL | blk(); | ----- `blk` moved due to this call LL | blk(); | ^^^ value used here after move | note: `FnOnce` closures can only be called once --> $DIR/once-cant-call-twice-on-heap.rs:6:10 | LL | fn foo(blk: F) { | ^^^^^^^^ `F` is made to be an `FnOnce` closure here LL | blk(); | ----- this value implements `FnOnce`, which causes it to be moved when called ``` --- compiler/rustc_borrowck/messages.ftl | 6 ++ .../src/diagnostics/conflict_errors.rs | 15 ++++- .../rustc_borrowck/src/diagnostics/mod.rs | 64 +++++++++++++++++-- .../borrowck/borrowck-unboxed-closures.stderr | 12 ++-- tests/ui/once-cant-call-twice-on-heap.stderr | 12 ++-- 5 files changed, 88 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_borrowck/messages.ftl b/compiler/rustc_borrowck/messages.ftl index 587536e1f9a..c14a617eb91 100644 --- a/compiler/rustc_borrowck/messages.ftl +++ b/compiler/rustc_borrowck/messages.ftl @@ -87,6 +87,12 @@ borrowck_move_unsized = borrowck_moved_a_fn_once_in_call = this value implements `FnOnce`, which causes it to be moved when called +borrowck_moved_a_fn_once_in_call_call = + `FnOnce` closures can only be called once + +borrowck_moved_a_fn_once_in_call_def = + `{$ty}` is made to be an `FnOnce` closure here + borrowck_moved_due_to_await = {$place_name} {$is_partial -> [true] partially moved diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 9abc29f52b2..d2d4345fed9 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -283,7 +283,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Some(name) => format!("`{name}`"), None => "value".to_owned(), }; - if self.suggest_borrow_fn_like(&mut err, ty, &move_site_vec, ¬e_msg) { + if self.suggest_borrow_fn_like(&mut err, ty, &move_site_vec, ¬e_msg) + || if let UseSpans::FnSelfUse { kind, .. } = use_spans + && let CallKind::FnCall { fn_trait_id, self_ty } = kind + && let ty::Param(_) = self_ty.kind() + && ty == self_ty + && Some(fn_trait_id) == self.infcx.tcx.lang_items().fn_once_trait() + { + // this is a type parameter `T: FnOnce()`, don't suggest `T: FnOnce() + Clone`. + true + } else { + false + } + { // Suppress the next suggestion since we don't want to put more bounds onto // something that already has `Fn`-like bounds (or is a closure), so we can't // restrict anyways. @@ -1016,7 +1028,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { None } }) - && { true } && parent_binop == other_parent_binop { // Explicitly look for `expr += other_expr;` and avoid suggesting diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 5425161ed48..53697b4b8b0 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -5,6 +5,7 @@ use crate::session_diagnostics::{ CaptureVarKind, CaptureVarPathUseCause, OnClosureNote, }; use rustc_errors::{Applicability, Diag}; +use rustc_errors::{DiagCtxt, MultiSpan}; use rustc_hir as hir; use rustc_hir::def::{CtorKind, Namespace}; use rustc_hir::CoroutineKind; @@ -29,6 +30,8 @@ use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::error_reporting::suggestions::TypeErrCtxtExt as _; use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions; +use crate::fluent_generated as fluent; + use super::borrow_set::BorrowData; use super::MirBorrowckCtxt; @@ -587,7 +590,7 @@ impl UseSpans<'_> { #[allow(rustc::diagnostic_outside_of_impl)] pub(super) fn args_subdiag( self, - dcx: &rustc_errors::DiagCtxt, + dcx: &DiagCtxt, err: &mut Diag<'_>, f: impl FnOnce(Span) -> CaptureArgLabel, ) { @@ -601,7 +604,7 @@ impl UseSpans<'_> { #[allow(rustc::diagnostic_outside_of_impl)] pub(super) fn var_path_only_subdiag( self, - dcx: &rustc_errors::DiagCtxt, + dcx: &DiagCtxt, err: &mut Diag<'_>, action: crate::InitializationRequiringAction, ) { @@ -639,7 +642,7 @@ impl UseSpans<'_> { #[allow(rustc::diagnostic_outside_of_impl)] pub(super) fn var_subdiag( self, - dcx: &rustc_errors::DiagCtxt, + dcx: &DiagCtxt, err: &mut Diag<'_>, kind: Option, f: impl FnOnce(hir::ClosureKind, Span) -> CaptureVarCause, @@ -1034,7 +1037,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { .map(|n| format!("`{n}`")) .unwrap_or_else(|| "value".to_owned()); match kind { - CallKind::FnCall { fn_trait_id, .. } + CallKind::FnCall { fn_trait_id, self_ty } if Some(fn_trait_id) == self.infcx.tcx.lang_items().fn_once_trait() => { err.subdiagnostic( @@ -1046,7 +1049,58 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { is_loop_message, }, ); - err.subdiagnostic(self.dcx(), CaptureReasonNote::FnOnceMoveInCall { var_span }); + if let ty::Param(param_ty) = self_ty.kind() + && let generics = self.infcx.tcx.generics_of(self.mir_def_id()) + && let param = generics.type_param(param_ty, self.infcx.tcx) + && let Some(hir_generics) = self + .infcx + .tcx + .typeck_root_def_id(self.mir_def_id().to_def_id()) + .as_local() + .and_then(|def_id| self.infcx.tcx.hir().get_generics(def_id)) + && let spans = hir_generics + .predicates + .iter() + .filter_map(|pred| match pred { + hir::WherePredicate::BoundPredicate(pred) => Some(pred), + _ => None, + }) + .filter(|pred| { + if let Some((id, _)) = pred.bounded_ty.as_generic_param() { + id == param.def_id + } else { + false + } + }) + .flat_map(|pred| pred.bounds) + .filter_map(|bound| { + if let Some(trait_ref) = bound.trait_ref() + && let Some(trait_def_id) = trait_ref.trait_def_id() + && trait_def_id == fn_trait_id + { + Some(bound.span()) + } else { + None + } + }) + .collect::>() + && !spans.is_empty() + { + let mut span: MultiSpan = spans.clone().into(); + for sp in spans { + span.push_span_label(sp, fluent::borrowck_moved_a_fn_once_in_call_def); + } + span.push_span_label( + fn_call_span, + fluent::borrowck_moved_a_fn_once_in_call, + ); + err.span_note(span, fluent::borrowck_moved_a_fn_once_in_call_call); + } else { + err.subdiagnostic( + self.dcx(), + CaptureReasonNote::FnOnceMoveInCall { var_span }, + ); + } } CallKind::Operator { self_arg, trait_id, .. } => { let self_arg = self_arg.unwrap(); diff --git a/tests/ui/borrowck/borrowck-unboxed-closures.stderr b/tests/ui/borrowck/borrowck-unboxed-closures.stderr index 3634676463c..a4513bd614e 100644 --- a/tests/ui/borrowck/borrowck-unboxed-closures.stderr +++ b/tests/ui/borrowck/borrowck-unboxed-closures.stderr @@ -29,15 +29,13 @@ LL | f(1, 2); LL | f(1, 2); | ^ value used here after move | -note: this value implements `FnOnce`, which causes it to be moved when called - --> $DIR/borrowck-unboxed-closures.rs:11:5 +note: `FnOnce` closures can only be called once + --> $DIR/borrowck-unboxed-closures.rs:10:8 | +LL | fn c isize>(f: F) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `F` is made to be an `FnOnce` closure here LL | f(1, 2); - | ^ -help: consider further restricting this bound - | -LL | fn c isize + Copy>(f: F) { - | ++++++ + | ------- this value implements `FnOnce`, which causes it to be moved when called error: aborting due to 3 previous errors diff --git a/tests/ui/once-cant-call-twice-on-heap.stderr b/tests/ui/once-cant-call-twice-on-heap.stderr index 33dd840dbc2..42697374115 100644 --- a/tests/ui/once-cant-call-twice-on-heap.stderr +++ b/tests/ui/once-cant-call-twice-on-heap.stderr @@ -8,15 +8,13 @@ LL | blk(); LL | blk(); | ^^^ value used here after move | -note: this value implements `FnOnce`, which causes it to be moved when called - --> $DIR/once-cant-call-twice-on-heap.rs:7:5 +note: `FnOnce` closures can only be called once + --> $DIR/once-cant-call-twice-on-heap.rs:6:10 | +LL | fn foo(blk: F) { + | ^^^^^^^^ `F` is made to be an `FnOnce` closure here LL | blk(); - | ^^^ -help: consider further restricting this bound - | -LL | fn foo(blk: F) { - | ++++++ + | ----- this value implements `FnOnce`, which causes it to be moved when called error: aborting due to 1 previous error