-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
noop_method_call: fix and improve derive suggestions
#134903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| LL - v.clone(); | ||
| LL + v; | ||
| | | ||
| help: if you meant to clone `non_clone_types::DifferentlyConditionalClone<PlainType<u8>>`, implement `Clone` for it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why this derive below is being suggested, since the bound is T: Default instead of T: Clone here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the problem is that cx.tcx.predicates_of(trait_impl_id) is actually empty in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with this, you can modify
.derive_suggestion = if you meant to clone `{$orig_ty}`, implement `Clone` for it
to be
.derive_suggestion = if you meant to clone `{$non_clone_ty}`, implement `Clone` for it
| LL - v.clone(); | ||
| LL + v; | ||
| | | ||
| help: if you meant to clone `non_clone_types::DifferentlyConditionalClone<PlainType<u8>>`, implement `Clone` for it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with this, you can modify
.derive_suggestion = if you meant to clone `{$orig_ty}`, implement `Clone` for it
to be
.derive_suggestion = if you meant to clone `{$non_clone_ty}`, implement `Clone` for it
… to derive on generic argument
7437998 to
ad93cb8
Compare
| ty::Adt(def, args) => { | ||
| if def.did().is_local() { | ||
| Some(cx.tcx.def_span(def.did()).shrink_to_lo()) | ||
| } else if let Some(trait_impl_id) = cx.tcx.impl_of_method(i.def_id()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i.def_id() is the wrong DefId, because it points to the Clone implementation for &T in core. How can I resolve the definition of T::clone (where T is orig_ty) instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use let args = tcx.mk_args(&[ty::GenericArg::from(orig_ty)]); for a new ty::Instance::try_resolve(cx.tcx, cx.typing_env(), did, args) call.
| LL + let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type; | ||
| | | ||
| help: if you meant to clone `PlainType<u32>`, implement `Clone` for it | ||
| help: if you meant to clone `PlainType<u32>`, implement `Clone` for `PlainType<u32>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be ideal if we didn't repeat the type when orig_ty == non_clone_ty.
|
☔ The latest upstream changes (presumably #136751) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Triage: comment is not addressed and need rebase |
Fixes #134471.
If
&Tis being cloned andTis not crate-local, stop suggesting#[derive(Clone)]forTin the message.If
&T<U>is being cloned andTis not crate-local, but implementsCloneifU: Clone, then try suggesting#[derive(Clone)]forUif it is local.r? estebank