Skip to content

Commit

Permalink
diagnostics: suggest Clone bounds when noop clone()
Browse files Browse the repository at this point in the history
  • Loading branch information
notriddle committed Mar 8, 2024
1 parent 9c3ad80 commit c2cc904
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -987,17 +987,46 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
else {
return false;
};
let arg_node = self.tcx.hir_node(*arg_hir_id);
let Node::Expr(Expr { kind: hir::ExprKind::Path(_), .. }) = arg_node else {
return false;
};

let clone_trait = self.tcx.require_lang_item(LangItem::Clone, None);
let has_clone = |ty| {
self.type_implements_trait(clone_trait, [ty], obligation.param_env)
.must_apply_modulo_regions()
};

let existing_clone_call = match self.tcx.hir_node(*arg_hir_id) {
// It's just a variable. Propose cloning it.
Node::Expr(Expr { kind: hir::ExprKind::Path(_), .. }) => None,
// It's already a call to `clone()`. We might be able to suggest
// adding a `+ Clone` bound, though.
Node::Expr(Expr {
kind:
hir::ExprKind::MethodCall(
hir::PathSegment { ident, .. },
_receiver,
&[],
call_span,
),
hir_id,
..
}) if ident.name == sym::clone
&& !call_span.from_expansion()
&& !has_clone(*inner_ty) =>
{
// We only care about method calls corresponding to the real `Clone` trait.
let Some(typeck_results) = self.typeck_results.as_ref() else { return false };
let Some((DefKind::AssocFn, did)) = typeck_results.type_dependent_def(*hir_id)
else {
return false;
};
if self.tcx.trait_of_item(did) != Some(clone_trait) {
return false;
}
Some(ident.span)
}
_ => return false,
};

let new_obligation = self.mk_trait_obligation_with_new_self_ty(
obligation.param_env,
trait_pred.map_bound(|trait_pred| (trait_pred, *inner_ty)),
Expand All @@ -1015,12 +1044,23 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
None,
);
}
err.span_suggestion_verbose(
obligation.cause.span.shrink_to_hi(),
"consider using clone here",
".clone()".to_string(),
Applicability::MaybeIncorrect,
);
if let Some(existing_clone_call) = existing_clone_call {
err.span_note(
existing_clone_call,
format!(
"this `clone()` copies the reference, \
which does not do anything, \
because `{inner_ty}` does not implement `Clone`"
),
);
} else {
err.span_suggestion_verbose(
obligation.cause.span.shrink_to_hi(),
"consider using clone here",
".clone()".to_string(),
Applicability::MaybeIncorrect,
);
}
return true;
}
false
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/suggestions/clone-bounds-121524.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#[derive(Clone)]
struct ThingThatDoesAThing;

trait DoesAThing {}

impl DoesAThing for ThingThatDoesAThing {}

fn clones_impl_ref_inline(thing: &impl DoesAThing) {
//~^ HELP consider further restricting this bound
drops_impl_owned(thing.clone()); //~ ERROR E0277
//~^ NOTE copies the reference
//~| NOTE the trait `DoesAThing` is not implemented for `&impl DoesAThing`
}

fn drops_impl_owned(_thing: impl DoesAThing) { }

fn main() {
clones_impl_ref_inline(&ThingThatDoesAThing);
}
19 changes: 19 additions & 0 deletions tests/ui/suggestions/clone-bounds-121524.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error[E0277]: the trait bound `&impl DoesAThing: DoesAThing` is not satisfied
--> $DIR/clone-bounds-121524.rs:10:22
|
LL | drops_impl_owned(thing.clone());
| ^^^^^^^^^^^^^ the trait `DoesAThing` is not implemented for `&impl DoesAThing`
|
note: this `clone()` copies the reference, which does not do anything, because `impl DoesAThing` does not implement `Clone`
--> $DIR/clone-bounds-121524.rs:10:28
|
LL | drops_impl_owned(thing.clone());
| ^^^^^
help: consider further restricting this bound
|
LL | fn clones_impl_ref_inline(thing: &impl DoesAThing + Clone) {
| +++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.

0 comments on commit c2cc904

Please sign in to comment.