diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index 2c7681c45a4..bdb6fc505f7 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -103,26 +103,39 @@ fn check_sig<'tcx>(cx: &LateContext<'tcx>, item_hir_id: hir::HirId, decl: &hir:: fn check_ty<'tcx>(cx: &LateContext<'tcx>, span: Span, ty: Ty<'tcx>) { let ty = ty.peel_refs(); if let Adt(def, substs) = ty.kind() { - if [sym::hashmap_type, sym::BTreeMap, sym::hashset_type, sym::BTreeMap] + let is_map_type = [sym::hashmap_type, sym::BTreeMap, sym::hashset_type, sym::BTreeMap] .iter() - .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did)) - && is_mutable_type(cx, substs.type_at(0), span) - { + .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did)); + if is_map_type && is_mutable_type(cx, substs.type_at(0), span, true) { span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type"); } } } -fn is_mutable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span) -> bool { +fn is_mutable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span, is_top_level_type: bool) -> bool { match *ty.kind() { - RawPtr(TypeAndMut { ty: inner_ty, mutbl }) | Ref(_, inner_ty, mutbl) => { - mutbl == hir::Mutability::Mut || is_mutable_type(cx, inner_ty, span) + RawPtr(TypeAndMut { ty: inner_ty, mutbl }) => { + if is_top_level_type { + // Raw pointers are hashed by the address they point to, not what is pointed to. + // Therefore, using a raw pointer to any type as the top-level key type is OK. + // Using raw pointers _in_ the key type is not, because the wrapper type could + // provide a custom `impl` for `Hash` (which could deref the raw pointer). + // + // see: + // - clippy issue: https://github.com/rust-lang/rust-clippy/issues/6745 + // - std code: https://github.com/rust-lang/rust/blob/1.54.0/library/core/src/hash/mod.rs#L717-L736 + false + } else { + mutbl == hir::Mutability::Mut || is_mutable_type(cx, inner_ty, span, false) + } }, - Slice(inner_ty) => is_mutable_type(cx, inner_ty, span), + Ref(_, inner_ty, mutbl) => mutbl == hir::Mutability::Mut || is_mutable_type(cx, inner_ty, span, false), + Slice(inner_ty) => is_mutable_type(cx, inner_ty, span, false), Array(inner_ty, size) => { - size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0) && is_mutable_type(cx, inner_ty, span) + size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0) + && is_mutable_type(cx, inner_ty, span, false) }, - Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span)), + Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span, false)), Adt(..) => { !ty.has_escaping_bound_vars() && cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() diff --git a/tests/ui/mut_key.rs b/tests/ui/mut_key.rs index 2d227e6654c..75b1618d82a 100644 --- a/tests/ui/mut_key.rs +++ b/tests/ui/mut_key.rs @@ -1,3 +1,4 @@ +use std::cell::Cell; use std::collections::{HashMap, HashSet}; use std::hash::{Hash, Hasher}; use std::sync::atomic::{AtomicUsize, Ordering::Relaxed}; @@ -31,11 +32,19 @@ fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> HashSet) {} +// Raw pointers are hashed by the address they point to, so it doesn't matter if they point to a +// type with interior mutability. See: +// - clippy issue: https://github.com/rust-lang/rust-clippy/issues/6745 +// - std lib: https://github.com/rust-lang/rust/blob/1.54.0/library/core/src/hash/mod.rs#L717-L736 +// So these are OK: +fn raw_ptr_is_ok(_m: &mut HashMap<*const Key, ()>) {} +fn raw_mut_ptr_is_ok(_m: &mut HashMap<*mut Key, ()>) {} + #[allow(unused)] trait Trait { type AssociatedType; - fn trait_fn(&self, set: std::collections::HashSet); + fn trait_fn(&self, set: HashSet); } fn generics_are_ok_too(_m: &mut HashSet) { @@ -52,4 +61,7 @@ fn main() { tuples::(&mut HashMap::new()); tuples::<()>(&mut HashMap::new()); tuples_bad::<()>(&mut HashMap::new()); + + raw_ptr_is_ok(&mut HashMap::new()); + raw_mut_ptr_is_ok(&mut HashMap::new()); } diff --git a/tests/ui/mut_key.stderr b/tests/ui/mut_key.stderr index a8460b06ca6..8da1262f0f1 100644 --- a/tests/ui/mut_key.stderr +++ b/tests/ui/mut_key.stderr @@ -1,5 +1,5 @@ error: mutable key type - --> $DIR/mut_key.rs:27:32 + --> $DIR/mut_key.rs:28:32 | LL | fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> HashSet { | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,19 +7,19 @@ LL | fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> Hash = note: `-D clippy::mutable-key-type` implied by `-D warnings` error: mutable key type - --> $DIR/mut_key.rs:27:72 + --> $DIR/mut_key.rs:28:72 | LL | fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> HashSet { | ^^^^^^^^^^^^ error: mutable key type - --> $DIR/mut_key.rs:28:5 + --> $DIR/mut_key.rs:29:5 | LL | let _other: HashMap = HashMap::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: mutable key type - --> $DIR/mut_key.rs:47:22 + --> $DIR/mut_key.rs:56:22 | LL | fn tuples_bad(_m: &mut HashMap<(Key, U), bool>) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^