diff options
| -rw-r--r-- | compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs | 113 | ||||
| -rw-r--r-- | tests/ui/borrowck/clone-on-ref.fixed | 32 | ||||
| -rw-r--r-- | tests/ui/borrowck/clone-on-ref.rs | 31 | ||||
| -rw-r--r-- | tests/ui/borrowck/clone-on-ref.stderr | 64 |
4 files changed, 233 insertions, 7 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 0776f455efd..fcbad472475 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -12,7 +12,6 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::{walk_block, walk_expr, Visitor}; use rustc_hir::{CoroutineDesugaring, PatField}; use rustc_hir::{CoroutineKind, CoroutineSource, LangItem}; -use rustc_infer::traits::ObligationCause; use rustc_middle::hir::nested_filter::OnlyBodies; use rustc_middle::mir::tcx::PlaceTy; use rustc_middle::mir::{ @@ -21,16 +20,21 @@ use rustc_middle::mir::{ PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, VarBindingForm, }; -use rustc_middle::ty::{self, suggest_constraining_type_params, PredicateKind, Ty, TyCtxt}; +use rustc_middle::ty::{ + self, suggest_constraining_type_params, PredicateKind, ToPredicate, Ty, TyCtxt, + TypeSuperVisitable, TypeVisitor, +}; use rustc_middle::util::CallKind; use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex}; +use rustc_span::def_id::DefId; use rustc_span::def_id::LocalDefId; use rustc_span::hygiene::DesugaringKind; use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::{BytePos, Span, Symbol}; use rustc_trait_selection::infer::InferCtxtExt; +use rustc_trait_selection::traits::error_reporting::suggestions::TypeErrCtxtExt; use rustc_trait_selection::traits::error_reporting::FindExprBySpan; -use rustc_trait_selection::traits::ObligationCtxt; +use rustc_trait_selection::traits::{Obligation, ObligationCause, ObligationCtxt}; use std::iter; use crate::borrow_set::TwoPhaseActivation; @@ -283,7 +287,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // something that already has `Fn`-like bounds (or is a closure), so we can't // restrict anyways. } else { - self.suggest_adding_copy_bounds(&mut err, ty, span); + let copy_did = self.infcx.tcx.require_lang_item(LangItem::Copy, Some(span)); + self.suggest_adding_bounds(&mut err, ty, copy_did, span); } if needs_note { @@ -775,7 +780,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - fn suggest_adding_copy_bounds(&self, err: &mut Diag<'_>, ty: Ty<'tcx>, span: Span) { + fn suggest_adding_bounds(&self, err: &mut Diag<'_>, ty: Ty<'tcx>, def_id: DefId, span: Span) { let tcx = self.infcx.tcx; let generics = tcx.generics_of(self.mir_def_id()); @@ -788,10 +793,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { }; // Try to find predicates on *generic params* that would allow copying `ty` let ocx = ObligationCtxt::new(self.infcx); - let copy_did = tcx.require_lang_item(LangItem::Copy, Some(span)); let cause = ObligationCause::misc(span, self.mir_def_id()); - ocx.register_bound(cause, self.param_env, ty, copy_did); + ocx.register_bound(cause, self.param_env, ty, def_id); let errors = ocx.select_all_or_error(); // Only emit suggestion if all required predicates are on generic @@ -877,6 +881,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Some(borrow_span), None, ); + self.suggest_copy_for_type_in_cloned_ref(&mut err, place); self.buffer_error(err); } @@ -1215,10 +1220,104 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); self.suggest_using_local_if_applicable(&mut err, location, issued_borrow, explanation); + self.suggest_copy_for_type_in_cloned_ref(&mut err, place); err } + fn suggest_copy_for_type_in_cloned_ref(&self, err: &mut Diag<'tcx>, place: Place<'tcx>) { + let tcx = self.infcx.tcx; + let hir = tcx.hir(); + let Some(body_id) = tcx.hir_node(self.mir_hir_id()).body_id() else { return }; + struct FindUselessClone<'hir> { + pub clones: Vec<&'hir hir::Expr<'hir>>, + } + impl<'hir> FindUselessClone<'hir> { + pub fn new() -> Self { + Self { clones: vec![] } + } + } + + impl<'v> Visitor<'v> for FindUselessClone<'v> { + fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { + // FIXME: use `lookup_method_for_diagnostic`? + if let hir::ExprKind::MethodCall(segment, _rcvr, args, _span) = ex.kind + && segment.ident.name == sym::clone + && args.len() == 0 + { + self.clones.push(ex); + } + hir::intravisit::walk_expr(self, ex); + } + } + let mut expr_finder = FindUselessClone::new(); + + let body = hir.body(body_id).value; + expr_finder.visit_expr(body); + + pub struct Holds<'tcx> { + ty: Ty<'tcx>, + holds: bool, + } + + impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for Holds<'tcx> { + type Result = std::ops::ControlFlow<()>; + + fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { + if t == self.ty { + self.holds = true; + } + t.super_visit_with(self) + } + } + + let mut types_to_constrain = FxIndexSet::default(); + + let local_ty = self.body.local_decls[place.local].ty; + let typeck_results = tcx.typeck(self.mir_def_id()); + let clone = tcx.require_lang_item(LangItem::Clone, Some(body.span)); + for expr in expr_finder.clones { + if let hir::ExprKind::MethodCall(_, rcvr, _, span) = expr.kind + && let Some(rcvr_ty) = typeck_results.node_type_opt(rcvr.hir_id) + && let Some(ty) = typeck_results.node_type_opt(expr.hir_id) + && rcvr_ty == ty + && let ty::Ref(_, inner, _) = rcvr_ty.kind() + && let inner = inner.peel_refs() + && let mut v = (Holds { ty: inner, holds: false }) + && let _ = v.visit_ty(local_ty) + && v.holds + && let None = self.infcx.type_implements_trait_shallow(clone, inner, self.param_env) + { + err.span_label( + span, + format!( + "this call doesn't do anything, the result is still `{rcvr_ty}` \ + because `{inner}` doesn't implement `Clone`", + ), + ); + types_to_constrain.insert(inner); + } + } + for ty in types_to_constrain { + self.suggest_adding_bounds(err, ty, clone, body.span); + if let ty::Adt(..) = ty.kind() { + // The type doesn't implement Clone. + let trait_ref = ty::Binder::dummy(ty::TraitRef::new(self.infcx.tcx, clone, [ty])); + let obligation = Obligation::new( + self.infcx.tcx, + ObligationCause::dummy(), + self.param_env, + trait_ref, + ); + self.infcx.err_ctxt().suggest_derive( + &obligation, + err, + trait_ref.to_predicate(self.infcx.tcx), + ); + } + } + } + #[instrument(level = "debug", skip(self, err))] fn suggest_using_local_if_applicable( &self, diff --git a/tests/ui/borrowck/clone-on-ref.fixed b/tests/ui/borrowck/clone-on-ref.fixed new file mode 100644 index 00000000000..b6927ba590e --- /dev/null +++ b/tests/ui/borrowck/clone-on-ref.fixed @@ -0,0 +1,32 @@ +//@ run-rustfix +fn foo<T: Default + Clone>(list: &mut Vec<T>) { + let mut cloned_items = Vec::new(); + for v in list.iter() { + cloned_items.push(v.clone()) + } + list.push(T::default()); + //~^ ERROR cannot borrow `*list` as mutable because it is also borrowed as immutable + drop(cloned_items); +} +fn bar<T: std::fmt::Display + Clone>(x: T) { + let a = &x; + let b = a.clone(); + drop(x); + //~^ ERROR cannot move out of `x` because it is borrowed + println!("{b}"); +} +#[derive(Debug)] +#[derive(Clone)] +struct A; +fn qux(x: A) { + let a = &x; + let b = a.clone(); + drop(x); + //~^ ERROR cannot move out of `x` because it is borrowed + println!("{b:?}"); +} +fn main() { + foo(&mut vec![1, 2, 3]); + bar(""); + qux(A); +} diff --git a/tests/ui/borrowck/clone-on-ref.rs b/tests/ui/borrowck/clone-on-ref.rs new file mode 100644 index 00000000000..f8c94d3cce3 --- /dev/null +++ b/tests/ui/borrowck/clone-on-ref.rs @@ -0,0 +1,31 @@ +//@ run-rustfix +fn foo<T: Default>(list: &mut Vec<T>) { + let mut cloned_items = Vec::new(); + for v in list.iter() { + cloned_items.push(v.clone()) + } + list.push(T::default()); + //~^ ERROR cannot borrow `*list` as mutable because it is also borrowed as immutable + drop(cloned_items); +} +fn bar<T: std::fmt::Display>(x: T) { + let a = &x; + let b = a.clone(); + drop(x); + //~^ ERROR cannot move out of `x` because it is borrowed + println!("{b}"); +} +#[derive(Debug)] +struct A; +fn qux(x: A) { + let a = &x; + let b = a.clone(); + drop(x); + //~^ ERROR cannot move out of `x` because it is borrowed + println!("{b:?}"); +} +fn main() { + foo(&mut vec![1, 2, 3]); + bar(""); + qux(A); +} diff --git a/tests/ui/borrowck/clone-on-ref.stderr b/tests/ui/borrowck/clone-on-ref.stderr new file mode 100644 index 00000000000..ee4fcadf55a --- /dev/null +++ b/tests/ui/borrowck/clone-on-ref.stderr @@ -0,0 +1,64 @@ +error[E0502]: cannot borrow `*list` as mutable because it is also borrowed as immutable + --> $DIR/clone-on-ref.rs:7:5 + | +LL | for v in list.iter() { + | ---- immutable borrow occurs here +LL | cloned_items.push(v.clone()) + | ------- this call doesn't do anything, the result is still `&T` because `T` doesn't implement `Clone` +LL | } +LL | list.push(T::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here +LL | +LL | drop(cloned_items); + | ------------ immutable borrow later used here + | +help: consider further restricting this bound + | +LL | fn foo<T: Default + Clone>(list: &mut Vec<T>) { + | +++++++ + +error[E0505]: cannot move out of `x` because it is borrowed + --> $DIR/clone-on-ref.rs:14:10 + | +LL | fn bar<T: std::fmt::Display>(x: T) { + | - binding `x` declared here +LL | let a = &x; + | -- borrow of `x` occurs here +LL | let b = a.clone(); + | ------- this call doesn't do anything, the result is still `&T` because `T` doesn't implement `Clone` +LL | drop(x); + | ^ move out of `x` occurs here +LL | +LL | println!("{b}"); + | --- borrow later used here + | +help: consider further restricting this bound + | +LL | fn bar<T: std::fmt::Display + Clone>(x: T) { + | +++++++ + +error[E0505]: cannot move out of `x` because it is borrowed + --> $DIR/clone-on-ref.rs:23:10 + | +LL | fn qux(x: A) { + | - binding `x` declared here +LL | let a = &x; + | -- borrow of `x` occurs here +LL | let b = a.clone(); + | ------- this call doesn't do anything, the result is still `&A` because `A` doesn't implement `Clone` +LL | drop(x); + | ^ move out of `x` occurs here +LL | +LL | println!("{b:?}"); + | ----- borrow later used here + | +help: consider annotating `A` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct A; + | + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0502, E0505. +For more information about an error, try `rustc --explain E0502`. |