Skip to content

Commit

Permalink
Avoid mut_key on types of unknown layout
Browse files Browse the repository at this point in the history
  • Loading branch information
llogiq committed Jan 19, 2020
1 parent f728bcd commit 59fd637
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 16 deletions.
25 changes: 14 additions & 11 deletions clippy_lints/src/mut_key.rs
@@ -1,5 +1,5 @@
use crate::utils::{match_def_path, paths, span_lint, trait_ref_of_method, walk_ptrs_ty};
use rustc::ty::{Adt, Dynamic, Opaque, Param, RawPtr, Ref, Ty, TypeAndMut};
use rustc::ty::{Adt, Array, RawPtr, Ref, Slice, Tuple, Ty, TypeAndMut};
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -101,21 +101,24 @@ fn check_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, ty: Ty<'tcx>) {
if [&paths::HASHMAP, &paths::BTREEMAP, &paths::HASHSET, &paths::BTREESET]
.iter()
.any(|path| match_def_path(cx, def.did, &**path))
&& is_mutable_type(cx, substs.type_at(0), span)
{
let key_type = concrete_type(substs.type_at(0));
if let Some(key_type) = key_type {
if !key_type.is_freeze(cx.tcx, cx.param_env, span) {
span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
}
}
span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
}
}
}

fn concrete_type(ty: Ty<'_>) -> Option<Ty<'_>> {
fn is_mutable_type<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>, span: Span) -> bool {
match ty.kind {
RawPtr(TypeAndMut { ty: inner_ty, .. }) | Ref(_, inner_ty, _) => concrete_type(inner_ty),
Dynamic(..) | Opaque(..) | Param(..) => None,
_ => Some(ty),
RawPtr(TypeAndMut { ty: inner_ty, mutbl }) | Ref(_, inner_ty, mutbl) => {
mutbl == hir::Mutability::Mut || is_mutable_type(cx, inner_ty, span)
},
Slice(inner_ty) => is_mutable_type(cx, inner_ty, span),
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)
},
Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span)),
Adt(..) => cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() && !ty.is_freeze(cx.tcx, cx.param_env, span),
_ => false,
}
}
22 changes: 21 additions & 1 deletion tests/ui/mut_key.rs
@@ -1,3 +1,5 @@
#![allow(clippy::implicit_hasher)]

use std::collections::{HashMap, HashSet};
use std::hash::{Hash, Hasher};
use std::sync::atomic::{AtomicUsize, Ordering::Relaxed};
Expand Down Expand Up @@ -29,9 +31,27 @@ fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<K
m.keys().cloned().collect()
}

fn this_is_ok(m: &mut HashMap<usize, Key>) {}
fn this_is_ok(_m: &mut HashMap<usize, Key>) {}

#[allow(unused)]
trait Trait {
type AssociatedType;

fn trait_fn(&self, set: std::collections::HashSet<Self::AssociatedType>);
}

fn generics_are_ok_too<K>(_m: &mut HashSet<K>) {
// nothing to see here, move along
}

fn tuples<U>(_m: &mut HashMap<((), U), ()>) {}

fn tuples_bad<U>(_m: &mut HashMap<(Key, U), bool>) {}

fn main() {
let _ = should_not_take_this_arg(&mut HashMap::new(), 1);
this_is_ok(&mut HashMap::new());
tuples::<Key>(&mut HashMap::new());
tuples::<()>(&mut HashMap::new());
tuples_bad::<()>(&mut HashMap::new());
}
14 changes: 10 additions & 4 deletions tests/ui/mut_key.stderr
@@ -1,22 +1,28 @@
error: mutable key type
--> $DIR/mut_key.rs:27:32
--> $DIR/mut_key.rs:29:32
|
LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<Key> {
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[deny(clippy::mutable_key_type)]` on by default

error: mutable key type
--> $DIR/mut_key.rs:27:72
--> $DIR/mut_key.rs:29:72
|
LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<Key> {
| ^^^^^^^^^^^^

error: mutable key type
--> $DIR/mut_key.rs:28:5
--> $DIR/mut_key.rs:30:5
|
LL | let _other: HashMap<Key, bool> = HashMap::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors
error: mutable key type
--> $DIR/mut_key.rs:49:22
|
LL | fn tuples_bad<U>(_m: &mut HashMap<(Key, U), bool>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 4 previous errors

0 comments on commit 59fd637

Please sign in to comment.