Skip to content

Commit 8c673e4

Browse files
committed
remove last statement special casing
1 parent a089c00 commit 8c673e4

6 files changed

+18
-47
lines changed

clippy_lints/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,6 +1172,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11721172
store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(msrv())));
11731173
store.register_early_pass(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers));
11741174
store.register_late_pass(|_| Box::new(zombie_processes::ZombieProcesses));
1175+
// add lints here, do not remove this comment, it's used in `new_lint`
11751176
}
11761177

11771178
#[rustfmt::skip]

clippy_lints/src/zombie_processes.rs

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -60,19 +60,18 @@ impl<'tcx> LateLintPass<'tcx> for ZombieProcesses {
6060
}
6161

6262
// Don't emit a suggestion since the binding is used later
63-
check(cx, expr, local.hir_id, false);
63+
check(cx, expr, false);
6464
},
65-
Node::LetStmt(&LetStmt { pat, hir_id, .. }) if let PatKind::Wild = pat.kind => {
65+
Node::LetStmt(&LetStmt { pat, .. }) if let PatKind::Wild = pat.kind => {
6666
// `let _ = child;`, also dropped immediately without `wait()`ing
67-
check(cx, expr, hir_id, true);
67+
check(cx, expr, true);
6868
},
6969
Node::Stmt(&Stmt {
7070
kind: StmtKind::Semi(_),
71-
hir_id,
7271
..
7372
}) => {
7473
// Immediately dropped. E.g. `std::process::Command::new("echo").spawn().unwrap();`
75-
check(cx, expr, hir_id, true);
74+
check(cx, expr, true);
7675
},
7776
_ => {},
7877
}
@@ -101,6 +100,7 @@ enum BreakReason {
101100
/// - calling `id` or `kill`
102101
/// - no use at all (e.g. `let _x = child;`)
103102
/// - taking a shared reference (`&`), `wait()` can't go through that
103+
///
104104
/// None of these are sufficient to prevent zombie processes.
105105
/// Doing it like this means more FNs, but FNs are better than FPs.
106106
///
@@ -212,9 +212,8 @@ impl<'a, 'tcx> Visitor<'tcx> for WaitFinder<'a, 'tcx> {
212212
/// such as checking that the binding is not used in certain ways, which isn't necessary for
213213
/// `let _ = <expr that spawns child>;`.
214214
///
215-
/// This checks if the program doesn't unconditionally exit after the spawn expression and that it
216-
/// isn't the last statement of the program.
217-
fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, node_id: HirId, emit_suggestion: bool) {
215+
/// This checks if the program doesn't unconditionally exit after the spawn expression.
216+
fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, emit_suggestion: bool) {
218217
let Some(block) = get_enclosing_block(cx, spawn_expr.hir_id) else {
219218
return;
220219
};
@@ -228,12 +227,6 @@ fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, node_id: Hi
228227
return;
229228
}
230229

231-
// This might be the last effective node of the program (main function).
232-
// There's no need to lint in that case either, as this is basically equivalent to calling `exit()`
233-
if is_last_node_in_main(cx, node_id) {
234-
return;
235-
}
236-
237230
span_lint_and_then(
238231
cx,
239232
ZOMBIE_PROCESSES,
@@ -257,26 +250,6 @@ fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, node_id: Hi
257250
);
258251
}
259252

260-
/// The hir id id may either correspond to a `Local` or `Stmt`, depending on how we got here.
261-
/// This function gets the enclosing function, checks if it's `main` and if so,
262-
/// check if the last statement modulo blocks is the given id.
263-
fn is_last_node_in_main(cx: &LateContext<'_>, id: HirId) -> bool {
264-
let hir = cx.tcx.hir();
265-
let body_owner = hir.enclosing_body_owner(id);
266-
let enclosing_body = hir.body_owned_by(body_owner);
267-
268-
if let Some((main_def_id, _)) = cx.tcx.entry_fn(())
269-
&& main_def_id == body_owner.to_def_id()
270-
&& let ExprKind::Block(block, _) = &enclosing_body.value.peel_blocks().kind
271-
&& let [.., stmt] = block.stmts
272-
{
273-
matches!(stmt.kind, StmtKind::Let(local) if local.hir_id == id)
274-
|| matches!(stmt.kind, StmtKind::Semi(..) if stmt.hir_id == id)
275-
} else {
276-
false
277-
}
278-
}
279-
280253
/// Checks if the given expression exits the process.
281254
fn is_exit_expression(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
282255
fn_def_id(cx, expr).is_some_and(|fn_did| {

tests/ui/zombie_processes.rs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,6 @@ fn main() {
131131
}
132132
x.wait().unwrap();
133133
}
134-
135-
// Checking that it won't lint if spawn is the last statement of a main function.
136-
// IMPORTANT: this case must always be at the very end so it always tests the right thing.
137-
// Don't move this.
138-
{
139-
{
140-
Command::new("").spawn().unwrap();
141-
}
142-
}
143134
}
144135

145136
fn process_child(c: Child) {

tests/ui/zombie_processes_fixable.fixed

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![warn(clippy::zombie_processes)]
2+
#![allow(clippy::needless_return)]
23

34
use std::process::{Child, Command};
45

@@ -19,3 +20,7 @@ fn not_main() {
1920
fn spawn_proc() -> Child {
2021
Command::new("").spawn().unwrap()
2122
}
23+
24+
fn spawn_proc_2() -> Child {
25+
return Command::new("").spawn().unwrap();
26+
}

tests/ui/zombie_processes_fixable.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![warn(clippy::zombie_processes)]
2+
#![allow(clippy::needless_return)]
23

34
use std::process::{Child, Command};
45

tests/ui/zombie_processes_fixable.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: spawned process is never `wait()`ed on
2-
--> tests/ui/zombie_processes_fixable.rs:6:13
2+
--> tests/ui/zombie_processes_fixable.rs:7:13
33
|
44
LL | let _ = Command::new("").spawn().unwrap();
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: try: `.wait()`
@@ -10,7 +10,7 @@ LL | let _ = Command::new("").spawn().unwrap();
1010
= help: to override `-D warnings` add `#[allow(clippy::zombie_processes)]`
1111

1212
error: spawned process is never `wait()`ed on
13-
--> tests/ui/zombie_processes_fixable.rs:8:5
13+
--> tests/ui/zombie_processes_fixable.rs:9:5
1414
|
1515
LL | Command::new("").spawn().unwrap();
1616
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: try: `.wait()`
@@ -19,7 +19,7 @@ LL | Command::new("").spawn().unwrap();
1919
= note: see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
2020

2121
error: spawned process is never `wait()`ed on
22-
--> tests/ui/zombie_processes_fixable.rs:10:5
22+
--> tests/ui/zombie_processes_fixable.rs:11:5
2323
|
2424
LL | spawn_proc();
2525
| ^^^^^^^^^^^^- help: try: `.wait()`
@@ -28,7 +28,7 @@ LL | spawn_proc();
2828
= note: see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
2929

3030
error: spawned process is never `wait()`ed on
31-
--> tests/ui/zombie_processes_fixable.rs:16:5
31+
--> tests/ui/zombie_processes_fixable.rs:17:5
3232
|
3333
LL | Command::new("").spawn().unwrap();
3434
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: try: `.wait()`

0 commit comments

Comments
 (0)