Skip to content

Commit

Permalink
Rollup merge of #112995 - strottos:ref-clone-suggestions, r=fee1-dead
Browse files Browse the repository at this point in the history 
Check for `<&NotClone as Clone>::clone()` calls and suggest to add Clone trait appropriately

Added recursive checking back up the HIR to see if a `Clone` suggestion would be helpful.

Addresses #112857

Largely based on: #112977
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Jul 25, 2023
2 parents 18fa7b9 + 25db1fa commit c5c0aa1
Show file tree
Hide file tree
Showing 3 changed files with 414 additions and 1 deletion.
84 changes: 84 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1523,9 +1523,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;
};
Expand Down Expand Up @@ -1578,6 +1584,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
Expand Down
116 changes: 116 additions & 0 deletions tests/ui/typeck/explain_clone_autoref.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
Loading

0 comments on commit c5c0aa1

Please sign in to comment.