From 25db1fac81103832519bd219877e7f756e690e77 Mon Sep 17 00:00:00 2001 From: Steven Trotter Date: Fri, 14 Jul 2023 21:33:45 +0100 Subject: [PATCH] Added recursive checking back up to see if a `Clone` suggestion would be helpful. --- .../src/fn_ctxt/suggestions.rs | 84 +++++++ tests/ui/typeck/explain_clone_autoref.rs | 116 ++++++++++ tests/ui/typeck/explain_clone_autoref.stderr | 215 +++++++++++++++++- 3 files changed, 414 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index ec19d017c25..77914a1c16c 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -1512,9 +1512,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { found_ty: Ty<'tcx>, expr: &hir::Expr<'_>, ) { + // When `expr` is `x` in something like `let x = foo.clone(); x`, need to recurse up to get + // `foo` and `clone`. + let expr = self.note_type_is_not_clone_inner_expr(expr); + + // If we've recursed to an `expr` of `foo.clone()`, get `foo` and `clone`. let hir::ExprKind::MethodCall(segment, callee_expr, &[], _) = expr.kind else { return; }; + let Some(clone_trait_did) = self.tcx.lang_items().clone_trait() else { return; }; @@ -1567,6 +1573,84 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + /// Given a type mismatch error caused by `&T` being cloned instead of `T`, and + /// the `expr` as the source of this type mismatch, try to find the method call + /// as the source of this error and return that instead. Otherwise, return the + /// original expression. + fn note_type_is_not_clone_inner_expr<'b>( + &'b self, + expr: &'b hir::Expr<'b>, + ) -> &'b hir::Expr<'b> { + match expr.peel_blocks().kind { + hir::ExprKind::Path(hir::QPath::Resolved( + None, + hir::Path { segments: [_], res: crate::Res::Local(binding), .. }, + )) => { + let Some(hir::Node::Pat(hir::Pat { hir_id, .. })) = self.tcx.hir().find(*binding) + else { + return expr; + }; + let Some(parent) = self.tcx.hir().find(self.tcx.hir().parent_id(*hir_id)) else { + return expr; + }; + + match parent { + // foo.clone() + hir::Node::Local(hir::Local { init: Some(init), .. }) => { + self.note_type_is_not_clone_inner_expr(init) + } + // When `expr` is more complex like a tuple + hir::Node::Pat(hir::Pat { + hir_id: pat_hir_id, + kind: hir::PatKind::Tuple(pats, ..), + .. + }) => { + let Some(hir::Node::Local(hir::Local { init: Some(init), .. })) = + self.tcx.hir().find(self.tcx.hir().parent_id(*pat_hir_id)) else { + return expr; + }; + + match init.peel_blocks().kind { + ExprKind::Tup(init_tup) => { + if let Some(init) = pats + .iter() + .enumerate() + .filter(|x| x.1.hir_id == *hir_id) + .map(|(i, _)| init_tup.get(i).unwrap()) + .next() + { + self.note_type_is_not_clone_inner_expr(init) + } else { + expr + } + } + _ => expr, + } + } + _ => expr, + } + } + // If we're calling into a closure that may not be typed recurse into that call. no need + // to worry if it's a call to a typed function or closure as this would ne handled + // previously. + hir::ExprKind::Call(Expr { kind: call_expr_kind, .. }, _) => { + if let hir::ExprKind::Path(hir::QPath::Resolved(None, call_expr_path)) = call_expr_kind + && let hir::Path { segments: [_], res: crate::Res::Local(binding), .. } = call_expr_path + && let Some(hir::Node::Pat(hir::Pat { hir_id, .. })) = self.tcx.hir().find(*binding) + && let Some(closure) = self.tcx.hir().find(self.tcx.hir().parent_id(*hir_id)) + && let hir::Node::Local(hir::Local { init: Some(init), .. }) = closure + && let Expr { kind: hir::ExprKind::Closure(hir::Closure { body: body_id, .. }), ..} = init + { + let hir::Body { value: body_expr, .. } = self.tcx.hir().body(*body_id); + self.note_type_is_not_clone_inner_expr(body_expr) + } else { + expr + } + } + _ => expr, + } + } + /// A common error is to add an extra semicolon: /// /// ```compile_fail,E0308 diff --git a/tests/ui/typeck/explain_clone_autoref.rs b/tests/ui/typeck/explain_clone_autoref.rs index 4d21574700a..88aaac469b2 100644 --- a/tests/ui/typeck/explain_clone_autoref.rs +++ b/tests/ui/typeck/explain_clone_autoref.rs @@ -11,3 +11,119 @@ fn clone_thing(nc: &NotClone) -> NotClone { //~| NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead //~| NOTE expected `NotClone`, found `&NotClone` } + +fn clone_thing2(nc: &NotClone) -> NotClone { + let nc: NotClone = nc.clone(); + //~^ ERROR mismatched type + //~| NOTE expected due to this + //~| NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + //~| NOTE expected `NotClone`, found `&NotClone` + nc +} + +fn clone_thing3(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let nc = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + nc + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing4(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let nc = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let nc2 = nc; + nc2 + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +impl NotClone { + fn other_fn(&self) {} + fn get_ref_notclone(&self) -> &Self { + self + } +} + +fn clone_thing5(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let nc = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let nc2 = nc; + nc2.other_fn(); + let nc3 = nc2; + nc3 + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing6(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let (ret, _) = (nc.clone(), 1); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let _ = nc.clone(); + ret + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing7(nc: Vec<&NotClone>) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let ret = nc[0].clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + ret + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing8(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let ret = { + let a = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + a + }; + ret + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing9(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let cl = || nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let ret = cl(); + ret + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing10(nc: &NotClone) -> (NotClone, NotClone) { + let (a, b) = { + let a = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + (a, nc.clone()) + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + }; + (a, b) + //~^ ERROR mismatched type + //~| ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` + //~| NOTE expected `NotClone`, found `&NotClone` +} + +fn clone_thing11(nc: &NotClone) -> NotClone { + //~^ NOTE expected `NotClone` because of return type + let a = { + let nothing = nc.clone(); + let a = nc.clone(); + //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + let nothing = nc.clone(); + a + }; + a + //~^ ERROR mismatched type + //~| NOTE expected `NotClone`, found `&NotClone` +} diff --git a/tests/ui/typeck/explain_clone_autoref.stderr b/tests/ui/typeck/explain_clone_autoref.stderr index 38cb7fe5518..40d3df30119 100644 --- a/tests/ui/typeck/explain_clone_autoref.stderr +++ b/tests/ui/typeck/explain_clone_autoref.stderr @@ -18,6 +18,219 @@ LL + #[derive(Clone)] LL | struct NotClone; | -error: aborting due to previous error +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:16:24 + | +LL | let nc: NotClone = nc.clone(); + | -------- ^^^^^^^^^^ expected `NotClone`, found `&NotClone` + | | + | expected due to this + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:16:24 + | +LL | let nc: NotClone = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:28:5 + | +LL | fn clone_thing3(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | nc + | ^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:26:14 + | +LL | let nc = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:38:5 + | +LL | fn clone_thing4(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | nc2 + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:35:14 + | +LL | let nc = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:57:5 + | +LL | fn clone_thing5(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | nc3 + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:52:14 + | +LL | let nc = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:67:5 + | +LL | fn clone_thing6(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | ret + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:64:21 + | +LL | let (ret, _) = (nc.clone(), 1); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:76:5 + | +LL | fn clone_thing7(nc: Vec<&NotClone>) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | ret + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:74:15 + | +LL | let ret = nc[0].clone(); + | ^^^^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:88:5 + | +LL | fn clone_thing8(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | ret + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:84:17 + | +LL | let a = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:98:5 + | +LL | fn clone_thing9(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | ret + | ^^^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:95:17 + | +LL | let cl = || nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:110:6 + | +LL | (a, b) + | ^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:105:17 + | +LL | let a = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:110:9 + | +LL | (a, b) + | ^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:107:13 + | +LL | (a, nc.clone()) + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error[E0308]: mismatched types + --> $DIR/explain_clone_autoref.rs:126:5 + | +LL | fn clone_thing11(nc: &NotClone) -> NotClone { + | -------- expected `NotClone` because of return type +... +LL | a + | ^ expected `NotClone`, found `&NotClone` + | +note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead + --> $DIR/explain_clone_autoref.rs:121:17 + | +LL | let a = nc.clone(); + | ^^ +help: consider annotating `NotClone` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NotClone; + | + +error: aborting due to 12 previous errors For more information about this error, try `rustc --explain E0308`.