Skip to content

Commit 12e96e8

Browse files
JarchoJarcho
committed
Minor cleanup of map_entry and a few additional tests.
1 parent 4de975c commit 12e96e8

File tree

9 files changed

+234
-216
lines changed

9 files changed

+234
-216
lines changed

‎clippy_lints/src/entry.rs

Lines changed: 95 additions & 92 deletions
Large diffs are not rendered by default.

‎clippy_utils/src/lib.rs

Lines changed: 14 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ use rustc_hir::def_id::{DefId, LOCAL_CRATE};
6161
use rustc_hir::intravisit::{self, walk_expr, ErasedMap, NestedVisitorMap, Visitor};
6262
use rustc_hir::{
6363
def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl,
64-
ImplItem, ImplItemKind, Item, ItemKind, LangItem, MatchSource, Node, Param, Pat, PatKind, Path, PathSegment, QPath,
65-
Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind,
64+
ImplItem, ImplItemKind, Item, ItemKind, LangItem, Local, MatchSource, Node, Param, Pat, PatKind, Path, PathSegment,
65+
QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind,
6666
};
6767
use rustc_lint::{LateContext, Level, Lint, LintContext};
6868
use rustc_middle::hir::exports::Export;
@@ -1301,48 +1301,7 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
13011301
did.map_or(false, |did| must_use_attr(&cx.tcx.get_attrs(did)).is_some())
13021302
}
13031303

1304-
pub fn get_expr_use_node(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<Node<'tcx>> {
1305-
let map = tcx.hir();
1306-
let mut child_id = expr.hir_id;
1307-
let mut iter = map.parent_iter(child_id);
1308-
loop {
1309-
match iter.next() {
1310-
None => break None,
1311-
Some((id, Node::Block(_))) => child_id = id,
1312-
Some((id, Node::Arm(arm))) if arm.body.hir_id == child_id => child_id = id,
1313-
Some((_, Node::Expr(expr))) => match expr.kind {
1314-
ExprKind::Break(
1315-
Destination {
1316-
target_id: Ok(dest), ..
1317-
},
1318-
_,
1319-
) => {
1320-
iter = map.parent_iter(dest);
1321-
child_id = dest;
1322-
},
1323-
ExprKind::DropTemps(_) | ExprKind::Block(..) => child_id = expr.hir_id,
1324-
ExprKind::If(control_expr, ..) | ExprKind::Match(control_expr, ..)
1325-
if control_expr.hir_id != child_id =>
1326-
{
1327-
child_id = expr.hir_id
1328-
},
1329-
_ => break Some(Node::Expr(expr)),
1330-
},
1331-
Some((_, node)) => break Some(node),
1332-
}
1333-
}
1334-
}
1335-
1336-
pub fn is_expr_used(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
1337-
!matches!(
1338-
get_expr_use_node(tcx, expr),
1339-
Some(Node::Stmt(Stmt {
1340-
kind: StmtKind::Expr(_) | StmtKind::Semi(_),
1341-
..
1342-
}))
1343-
)
1344-
}
1345-
1304+
/// Gets the node where an expression is either used, or it's type is unified with another branch.
13461305
pub fn get_expr_use_or_unification_node(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<Node<'tcx>> {
13471306
let map = tcx.hir();
13481307
let mut child_id = expr.hir_id;
@@ -1363,16 +1322,26 @@ pub fn get_expr_use_or_unification_node(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> O
13631322
}
13641323
}
13651324

1325+
/// Checks if the result of an expression is used, or it's type is unified with another branch.
13661326
pub fn is_expr_used_or_unified(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
13671327
!matches!(
13681328
get_expr_use_or_unification_node(tcx, expr),
13691329
None | Some(Node::Stmt(Stmt {
1370-
kind: StmtKind::Expr(_) | StmtKind::Semi(_),
1330+
kind: StmtKind::Expr(_)
1331+
| StmtKind::Semi(_)
1332+
| StmtKind::Local(Local {
1333+
pat: Pat {
1334+
kind: PatKind::Wild,
1335+
..
1336+
},
1337+
..
1338+
}),
13711339
..
13721340
}))
13731341
)
13741342
}
13751343

1344+
/// Checks if the expression is the final expression returned from a block.
13761345
pub fn is_expr_final_block_expr(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
13771346
matches!(get_parent_node(tcx, expr.hir_id), Some(Node::Block(..)))
13781347
}

‎tests/ui/entry.fixed

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![allow(unused, clippy::needless_pass_by_value, clippy::collapsible_if)]
44
#![warn(clippy::map_entry)]
5+
#![feature(asm)]
56

67
use std::collections::{BTreeMap, HashMap};
78
use std::hash::Hash;
@@ -10,11 +11,19 @@ macro_rules! m {
1011
($e:expr) => {{ $e }};
1112
}
1213

14+
macro_rules! insert {
15+
($map:expr, $key:expr, $val:expr) => {
16+
$map.insert($key, $val)
17+
};
18+
}
19+
1320
fn foo() {}
1421

15-
fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2: V) {
22+
fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, m2: &mut HashMap<K, V>, k: K, k2: K, v: V, v2: V) {
23+
// or_insert(v)
1624
m.entry(k).or_insert(v);
1725

26+
// semicolon on insert, use or_insert_with(..)
1827
m.entry(k).or_insert_with(|| {
1928
if true {
2029
v
@@ -23,6 +32,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
2332
}
2433
});
2534

35+
// semicolon on if, use or_insert_with(..)
2636
m.entry(k).or_insert_with(|| {
2737
if true {
2838
v
@@ -31,6 +41,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
3141
}
3242
});
3343

44+
// early return, use if let
3445
if let std::collections::hash_map::Entry::Vacant(e) = m.entry(k) {
3546
if true {
3647
e.insert(v);
@@ -40,11 +51,13 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
4051
}
4152
}
4253

54+
// use or_insert_with(..)
4355
m.entry(k).or_insert_with(|| {
4456
foo();
4557
v
4658
});
4759

60+
// semicolon on insert and match, use or_insert_with(..)
4861
m.entry(k).or_insert_with(|| {
4962
match 0 {
5063
1 if true => {
@@ -56,18 +69,17 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
5669
}
5770
});
5871

72+
// one branch doesn't insert, use if let
5973
if let std::collections::hash_map::Entry::Vacant(e) = m.entry(k) {
6074
match 0 {
61-
0 => {},
62-
1 => {
63-
e.insert(v);
64-
},
75+
0 => foo(),
6576
_ => {
6677
e.insert(v2);
6778
},
6879
};
6980
}
7081

82+
// use or_insert_with
7183
m.entry(k).or_insert_with(|| {
7284
foo();
7385
match 0 {
@@ -94,10 +106,46 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
94106
}
95107
});
96108

109+
// ok, insert in loop
110+
if !m.contains_key(&k) {
111+
for _ in 0..2 {
112+
m.insert(k, v);
113+
}
114+
}
115+
116+
// macro_expansion test, use or_insert(..)
97117
m.entry(m!(k)).or_insert_with(|| m!(v));
118+
119+
// ok, map used before insertion
120+
if !m.contains_key(&k) {
121+
let _ = m.len();
122+
m.insert(k, v);
123+
}
124+
125+
// ok, inline asm
126+
if !m.contains_key(&k) {
127+
unsafe { asm!("nop") }
128+
m.insert(k, v);
129+
}
130+
131+
// ok, different keys.
132+
if !m.contains_key(&k) {
133+
m.insert(k2, v);
134+
}
135+
136+
// ok, different maps
137+
if !m.contains_key(&k) {
138+
m2.insert(k, v);
139+
}
140+
141+
// ok, insert in macro
142+
if !m.contains_key(&k) {
143+
insert!(m, k, v);
144+
}
98145
}
99146

100147
fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V, v2: V) {
148+
// insert then do something, use if let
101149
if let std::collections::btree_map::Entry::Vacant(e) = m.entry(k) {
102150
e.insert(v);
103151
foo();

‎tests/ui/entry.rs

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![allow(unused, clippy::needless_pass_by_value, clippy::collapsible_if)]
44
#![warn(clippy::map_entry)]
5+
#![feature(asm)]
56

67
use std::collections::{BTreeMap, HashMap};
78
use std::hash::Hash;
@@ -10,13 +11,21 @@ macro_rules! m {
1011
($e:expr) => {{ $e }};
1112
}
1213

14+
macro_rules! insert {
15+
($map:expr, $key:expr, $val:expr) => {
16+
$map.insert($key, $val)
17+
};
18+
}
19+
1320
fn foo() {}
1421

15-
fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2: V) {
22+
fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, m2: &mut HashMap<K, V>, k: K, k2: K, v: V, v2: V) {
23+
// or_insert(v)
1624
if !m.contains_key(&k) {
1725
m.insert(k, v);
1826
}
1927

28+
// semicolon on insert, use or_insert_with(..)
2029
if !m.contains_key(&k) {
2130
if true {
2231
m.insert(k, v);
@@ -25,6 +34,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
2534
}
2635
}
2736

37+
// semicolon on if, use or_insert_with(..)
2838
if !m.contains_key(&k) {
2939
if true {
3040
m.insert(k, v)
@@ -33,6 +43,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
3343
};
3444
}
3545

46+
// early return, use if let
3647
if !m.contains_key(&k) {
3748
if true {
3849
m.insert(k, v);
@@ -42,11 +53,13 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
4253
}
4354
}
4455

56+
// use or_insert_with(..)
4557
if !m.contains_key(&k) {
4658
foo();
4759
m.insert(k, v);
4860
}
4961

62+
// semicolon on insert and match, use or_insert_with(..)
5063
if !m.contains_key(&k) {
5164
match 0 {
5265
1 if true => {
@@ -58,18 +71,17 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
5871
};
5972
}
6073

74+
// one branch doesn't insert, use if let
6175
if !m.contains_key(&k) {
6276
match 0 {
63-
0 => {},
64-
1 => {
65-
m.insert(k, v);
66-
},
77+
0 => foo(),
6778
_ => {
6879
m.insert(k, v2);
6980
},
7081
};
7182
}
7283

84+
// use or_insert_with
7385
if !m.contains_key(&k) {
7486
foo();
7587
match 0 {
@@ -96,12 +108,48 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
96108
}
97109
}
98110

111+
// ok, insert in loop
112+
if !m.contains_key(&k) {
113+
for _ in 0..2 {
114+
m.insert(k, v);
115+
}
116+
}
117+
118+
// macro_expansion test, use or_insert(..)
99119
if !m.contains_key(&m!(k)) {
100120
m.insert(m!(k), m!(v));
101121
}
122+
123+
// ok, map used before insertion
124+
if !m.contains_key(&k) {
125+
let _ = m.len();
126+
m.insert(k, v);
127+
}
128+
129+
// ok, inline asm
130+
if !m.contains_key(&k) {
131+
unsafe { asm!("nop") }
132+
m.insert(k, v);
133+
}
134+
135+
// ok, different keys.
136+
if !m.contains_key(&k) {
137+
m.insert(k2, v);
138+
}
139+
140+
// ok, different maps
141+
if !m.contains_key(&k) {
142+
m2.insert(k, v);
143+
}
144+
145+
// ok, insert in macro
146+
if !m.contains_key(&k) {
147+
insert!(m, k, v);
148+
}
102149
}
103150

104151
fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V, v2: V) {
152+
// insert then do something, use if let
105153
if !m.contains_key(&k) {
106154
m.insert(k, v);
107155
foo();

0 commit comments

Comments
 (0)