Skip to content

Commit

Permalink
print cause chain in mutable_key_type
Browse files Browse the repository at this point in the history
  • Loading branch information
y21 committed Oct 3, 2024
1 parent db1bda3 commit 19d1358
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 23 deletions.
13 changes: 10 additions & 3 deletions clippy_lints/src/mut_key.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::trait_ref_of_method;
use clippy_utils::ty::InteriorMut;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::impl_lint_pass;
use rustc_span::Span;
Expand Down Expand Up @@ -132,8 +133,14 @@ impl<'tcx> MutableKeyType<'tcx> {
)
{
let subst_ty = args.type_at(0);
if self.interior_mut.is_interior_mut_ty(cx, subst_ty) {
span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
if let Some(chain) = self.interior_mut.interior_mut_ty_chain(cx, subst_ty) {
span_lint_and_then(cx, MUTABLE_KEY_TYPE, span, "mutable key type", |diag| {
for ty in chain.iter().rev() {
diag.note(with_forced_trimmed_paths!(format!(
"... because it contains `{ty}`, which has interior mutability"
)));
}
});
}
}
}
Expand Down
48 changes: 28 additions & 20 deletions clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1168,7 +1168,7 @@ pub fn make_normalized_projection<'tcx>(
pub struct InteriorMut<'tcx> {
ignored_def_ids: FxHashSet<DefId>,
ignore_pointers: bool,
tys: FxHashMap<Ty<'tcx>, Option<bool>>,
tys: FxHashMap<Ty<'tcx>, Option<&'tcx ty::List<Ty<'tcx>>>>,
}

impl<'tcx> InteriorMut<'tcx> {
Expand All @@ -1194,25 +1194,24 @@ impl<'tcx> InteriorMut<'tcx> {
}
}

/// Check if given type has inner mutability such as [`std::cell::Cell`] or
/// [`std::cell::RefCell`] etc.
pub fn is_interior_mut_ty(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
/// Check if given type has interior mutability such as [`std::cell::Cell`] or
/// [`std::cell::RefCell`] etc. and if it does, returns a chain of types that causes
/// this type to be interior mutable
pub fn interior_mut_ty_chain(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'tcx ty::List<Ty<'tcx>>> {
match self.tys.entry(ty) {
Entry::Occupied(o) => return *o.get() == Some(true),
Entry::Occupied(o) => return *o.get(),
// Temporarily insert a `None` to break cycles
Entry::Vacant(v) => v.insert(None),
};

let interior_mut = match *ty.kind() {
ty::RawPtr(inner_ty, _) if !self.ignore_pointers => self.is_interior_mut_ty(cx, inner_ty),
ty::Ref(_, inner_ty, _) | ty::Slice(inner_ty) => self.is_interior_mut_ty(cx, inner_ty),
ty::Array(inner_ty, size) => {
size.try_eval_target_usize(cx.tcx, cx.param_env)
.map_or(true, |u| u != 0)
&& self.is_interior_mut_ty(cx, inner_ty)
let chain = match *ty.kind() {
ty::RawPtr(inner_ty, _) if !self.ignore_pointers => self.interior_mut_ty_chain(cx, inner_ty),
ty::Ref(_, inner_ty, _) | ty::Slice(inner_ty) => self.interior_mut_ty_chain(cx, inner_ty),
ty::Array(inner_ty, size) if size.try_eval_target_usize(cx.tcx, cx.param_env) != Some(0) => {
self.interior_mut_ty_chain(cx, inner_ty)
},
ty::Tuple(fields) => fields.iter().any(|ty| self.is_interior_mut_ty(cx, ty)),
ty::Adt(def, _) if def.is_unsafe_cell() => true,
ty::Tuple(fields) => fields.iter().find_map(|ty| self.interior_mut_ty_chain(cx, ty)),
ty::Adt(def, _) if def.is_unsafe_cell() => Some(ty::List::empty()),
ty::Adt(def, args) => {
let is_std_collection = matches!(
cx.tcx.get_diagnostic_name(def.did()),
Expand All @@ -1231,19 +1230,28 @@ impl<'tcx> InteriorMut<'tcx> {

if is_std_collection || def.is_box() {
// Include the types from std collections that are behind pointers internally
args.types().any(|ty| self.is_interior_mut_ty(cx, ty))
args.types().find_map(|ty| self.interior_mut_ty_chain(cx, ty))
} else if self.ignored_def_ids.contains(&def.did()) || def.is_phantom_data() {
false
None
} else {
def.all_fields()
.any(|f| self.is_interior_mut_ty(cx, f.ty(cx.tcx, args)))
.find_map(|f| self.interior_mut_ty_chain(cx, f.ty(cx.tcx, args)))
}
},
_ => false,
_ => None,
};

self.tys.insert(ty, Some(interior_mut));
interior_mut
chain.map(|chain| {
let list = cx.tcx.mk_type_list_from_iter(chain.iter().chain([ty]));
self.tys.insert(ty, Some(list));
list
})
}

/// Check if given type has interior mutability such as [`std::cell::Cell`] or
/// [`std::cell::RefCell`] etc.
pub fn is_interior_mut_ty(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
self.interior_mut_ty_chain(cx, ty).is_some()
}
}

Expand Down
60 changes: 60 additions & 0 deletions tests/ui/mut_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ error: mutable key type
LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<Key> {
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `Key`, which has interior mutability
= note: ... because it contains `AtomicUsize`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability
= note: `-D clippy::mutable-key-type` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::mutable_key_type)]`

Expand All @@ -12,84 +15,141 @@ error: mutable key type
|
LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<Key> {
| ^^^^^^^^^^^^
|
= note: ... because it contains `Key`, which has interior mutability
= note: ... because it contains `AtomicUsize`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:35:5
|
LL | let _other: HashMap<Key, bool> = HashMap::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `Key`, which has interior mutability
= note: ... because it contains `AtomicUsize`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:63:22
|
LL | fn tuples_bad<U>(_m: &mut HashMap<(Key, U), bool>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `(Key, U)`, which has interior mutability
= note: ... because it contains `Key`, which has interior mutability
= note: ... because it contains `AtomicUsize`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:76:5
|
LL | let _map = HashMap::<Cell<usize>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:78:5
|
LL | let _map = HashMap::<&mut Cell<usize>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `&mut Cell<usize>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:81:5
|
LL | let _map = HashMap::<Vec<Cell<usize>>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `Vec<Cell<usize>>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:83:5
|
LL | let _map = HashMap::<BTreeMap<Cell<usize>, ()>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `BTreeMap<Cell<usize>, ()>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:85:5
|
LL | let _map = HashMap::<BTreeMap<(), Cell<usize>>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `BTreeMap<(), Cell<usize>>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:87:5
|
LL | let _map = HashMap::<BTreeSet<Cell<usize>>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `BTreeSet<Cell<usize>>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:89:5
|
LL | let _map = HashMap::<Option<Cell<usize>>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `Option<Cell<usize>>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:91:5
|
LL | let _map = HashMap::<Option<Vec<Cell<usize>>>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `Option<Vec<Cell<usize>>>`, which has interior mutability
= note: ... because it contains `Vec<Cell<usize>>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:94:5
|
LL | let _map = HashMap::<Box<Cell<usize>>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `Box<Cell<usize>>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:96:5
|
LL | let _map = HashMap::<Rc<Cell<usize>>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `Rc<Cell<usize>>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: mutable key type
--> tests/ui/mut_key.rs:98:5
|
LL | let _map = HashMap::<Arc<Cell<usize>>, usize>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ... because it contains `Arc<Cell<usize>>`, which has interior mutability
= note: ... because it contains `Cell<usize>`, which has interior mutability
= note: ... because it contains `UnsafeCell<usize>`, which has interior mutability

error: aborting due to 15 previous errors

0 comments on commit 19d1358

Please sign in to comment.