Make ops::DerefMut a diagnostic item#147643
Conversation
rustbot
added
S-waiting-on-review
|
@relaxcn Why would it be a good idea to allow such calls in Deref(Mut) impls? |
Reference to rust-lang/rust-clippy#15392, I think it is make sense to me. |
| @@ -266,6 +266,7 @@ impl<T: ?Sized> const Deref for &mut T { | |||
| #[lang = "deref_mut"] | |||
There was a problem hiding this comment.
DerefMut already has a lang item, can you use LangItem::DeferMut instead?
There was a problem hiding this comment.
Of course. I can use it to archive the same goal.
But strangely enough, Deref is both a language item and a diagnostic item:
rust/library/core/src/ops/deref.rs
Lines 133 to 139 in 4b94758
Why not the DerefMut to be a diagnostic item? Then we can use them uniformly like:
[sym::Deref, sym::DerefMut]
.iter()
.any(|&sym| cx.tcx.is_diagnostic_item(sym, trait_id))
There was a problem hiding this comment.
this feels like a reason to remove the Deref diagnostics item as well 😆
There was a problem hiding this comment.
😂 I'm just confused about it.
Is it bad to use diagnostics item?
There was a problem hiding this comment.
no worries
I am actually not quite sure. Our general approach is to use lang_items instead of diagnostics items if they already exist.
I don't think there's a large reason for that and we may even want is_diagnostics_item to also just lookup lang items 😁 actually, I think that would be a good improvement
The main reason seems to be "there should only be one consistent way to do things"
There was a problem hiding this comment.
Thank you for your explanation.
So I will close this PR.
BTW it will have a big influence if we remove Deref diagnostics item because rust-clippy use it.
There was a problem hiding this comment.
we have clippy as a subtree, so we can update it in the same PR
There was a problem hiding this comment.
All right. Thank you.
rustbot
removed
the
S-waiting-on-review
5 participants
I'm working on an enhancement to clippy::explicit_deref_methods, which would allow explicit
dereforderef_mutmethod calls in implementation of the theDereforDerefMuttrait, so I need aDerefMutdiagnostic item to check if we are already in the impl ofDerefMuttrait.