Skip to content

Commit 7c3908f

Browse files
y21y21
committedFeb 4, 2024
[redundant_locals]: take by-value closure captures into account
1 parent 9fb4107 commit 7c3908f

File tree

3 files changed

+93
-29
lines changed

3 files changed

+93
-29
lines changed
 

Diff for: ‎clippy_lints/src/redundant_locals.rs

+29-1
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::is_from_proc_macro;
33
use clippy_utils::ty::needs_ordered_drop;
44
use rustc_ast::Mutability;
5-
use rustc_hir::def::Res;
5+
use rustc_hir::def::{DefKind, Res};
66
use rustc_hir::{BindingAnnotation, ByRef, ExprKind, HirId, Local, Node, Pat, PatKind, QPath};
7+
use rustc_hir_typeck::expr_use_visitor::PlaceBase;
78
use rustc_lint::{LateContext, LateLintPass, LintContext};
89
use rustc_middle::lint::in_external_macro;
10+
use rustc_middle::ty::UpvarCapture;
911
use rustc_session::declare_lint_pass;
1012
use rustc_span::symbol::Ident;
1113
use rustc_span::DesugaringKind;
@@ -69,6 +71,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantLocals {
6971
// the local is user-controlled
7072
&& !in_external_macro(cx.sess(), local.span)
7173
&& !is_from_proc_macro(cx, expr)
74+
&& !is_closure_capture(cx, local.hir_id, binding_id)
7275
{
7376
span_lint_and_help(
7477
cx,
@@ -82,6 +85,31 @@ impl<'tcx> LateLintPass<'tcx> for RedundantLocals {
8285
}
8386
}
8487

88+
/// Checks if the enclosing body is a closure and if the given local is captured by value.
89+
///
90+
/// In those cases, the redefinition may be necessary to force a move:
91+
/// ```
92+
/// fn assert_static<T: 'static>(_: T) {}
93+
///
94+
/// let v = String::new();
95+
/// let closure = || {
96+
/// let v = v; // <- removing this redefinition makes `closure` no longer `'static`
97+
/// dbg!(&v);
98+
/// };
99+
/// assert_static(closure);
100+
/// ```
101+
fn is_closure_capture(cx: &LateContext<'_>, redefinition: HirId, root_variable: HirId) -> bool {
102+
let body = cx.tcx.hir().enclosing_body_owner(redefinition);
103+
if let DefKind::Closure = cx.tcx.def_kind(body) {
104+
cx.tcx.closure_captures(body).iter().any(|c| {
105+
matches!(c.info.capture_kind, UpvarCapture::ByValue)
106+
&& matches!(c.place.base, PlaceBase::Upvar(upvar) if upvar.var_path.hir_id == root_variable)
107+
})
108+
} else {
109+
false
110+
}
111+
}
112+
85113
/// Find the annotation of a binding introduced by a pattern, or `None` if it's not introduced.
86114
fn find_binding(pat: &Pat<'_>, name: Ident) -> Option<BindingAnnotation> {
87115
let mut ret = None;

Diff for: ‎tests/ui/redundant_locals.rs

+36
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//@aux-build:proc_macros.rs
22
#![allow(unused, clippy::no_effect, clippy::needless_pass_by_ref_mut)]
33
#![warn(clippy::redundant_locals)]
4+
#![feature(async_closure)]
45

56
extern crate proc_macros;
67
use proc_macros::{external, with_span};
@@ -163,3 +164,38 @@ fn drop_compose() {
163164
let b = ComposeDrop { d: WithDrop(1) };
164165
let a = a;
165166
}
167+
168+
fn issue12225() {
169+
fn assert_static<T: 'static>(_: T) {}
170+
171+
let v1 = String::new();
172+
let v2 = String::new();
173+
let v3 = String::new();
174+
let v4 = String::new();
175+
176+
assert_static(|| {
177+
let v1 = v1;
178+
dbg!(&v1);
179+
});
180+
assert_static(async {
181+
let v2 = v2;
182+
dbg!(&v2);
183+
});
184+
assert_static(|| async {
185+
let v3 = v3;
186+
dbg!(&v3);
187+
});
188+
assert_static(async || {
189+
let v4 = v4;
190+
dbg!(&v4);
191+
});
192+
193+
fn foo(a: &str, b: &str) {}
194+
195+
let do_not_move = String::new();
196+
let things_to_move = vec!["a".to_string(), "b".to_string()];
197+
let futures = things_to_move.into_iter().map(|move_me| async {
198+
let move_me = move_me;
199+
foo(&do_not_move, &move_me)
200+
});
201+
}

Diff for: ‎tests/ui/redundant_locals.stderr

+28-28
Original file line numberDiff line numberDiff line change
@@ -1,169 +1,169 @@
11
error: redundant redefinition of a binding `x`
2-
--> $DIR/redundant_locals.rs:12:5
2+
--> $DIR/redundant_locals.rs:13:5
33
|
44
LL | let x = x;
55
| ^^^^^^^^^^
66
|
77
help: `x` is initially defined here
8-
--> $DIR/redundant_locals.rs:11:9
8+
--> $DIR/redundant_locals.rs:12:9
99
|
1010
LL | let x = 1;
1111
| ^
1212
= note: `-D clippy::redundant-locals` implied by `-D warnings`
1313
= help: to override `-D warnings` add `#[allow(clippy::redundant_locals)]`
1414

1515
error: redundant redefinition of a binding `x`
16-
--> $DIR/redundant_locals.rs:17:5
16+
--> $DIR/redundant_locals.rs:18:5
1717
|
1818
LL | let mut x = x;
1919
| ^^^^^^^^^^^^^^
2020
|
2121
help: `x` is initially defined here
22-
--> $DIR/redundant_locals.rs:16:9
22+
--> $DIR/redundant_locals.rs:17:9
2323
|
2424
LL | let mut x = 1;
2525
| ^^^^^
2626

2727
error: redundant redefinition of a binding `x`
28-
--> $DIR/redundant_locals.rs:47:5
28+
--> $DIR/redundant_locals.rs:48:5
2929
|
3030
LL | let x = x;
3131
| ^^^^^^^^^^
3232
|
3333
help: `x` is initially defined here
34-
--> $DIR/redundant_locals.rs:46:14
34+
--> $DIR/redundant_locals.rs:47:14
3535
|
3636
LL | fn parameter(x: i32) {
3737
| ^
3838

3939
error: redundant redefinition of a binding `x`
40-
--> $DIR/redundant_locals.rs:52:5
40+
--> $DIR/redundant_locals.rs:53:5
4141
|
4242
LL | let x = x;
4343
| ^^^^^^^^^^
4444
|
4545
help: `x` is initially defined here
46-
--> $DIR/redundant_locals.rs:51:9
46+
--> $DIR/redundant_locals.rs:52:9
4747
|
4848
LL | let x = 1;
4949
| ^
5050

5151
error: redundant redefinition of a binding `x`
52-
--> $DIR/redundant_locals.rs:53:5
52+
--> $DIR/redundant_locals.rs:54:5
5353
|
5454
LL | let x = x;
5555
| ^^^^^^^^^^
5656
|
5757
help: `x` is initially defined here
58-
--> $DIR/redundant_locals.rs:52:9
58+
--> $DIR/redundant_locals.rs:53:9
5959
|
6060
LL | let x = x;
6161
| ^
6262

6363
error: redundant redefinition of a binding `x`
64-
--> $DIR/redundant_locals.rs:54:5
64+
--> $DIR/redundant_locals.rs:55:5
6565
|
6666
LL | let x = x;
6767
| ^^^^^^^^^^
6868
|
6969
help: `x` is initially defined here
70-
--> $DIR/redundant_locals.rs:53:9
70+
--> $DIR/redundant_locals.rs:54:9
7171
|
7272
LL | let x = x;
7373
| ^
7474

7575
error: redundant redefinition of a binding `x`
76-
--> $DIR/redundant_locals.rs:55:5
76+
--> $DIR/redundant_locals.rs:56:5
7777
|
7878
LL | let x = x;
7979
| ^^^^^^^^^^
8080
|
8181
help: `x` is initially defined here
82-
--> $DIR/redundant_locals.rs:54:9
82+
--> $DIR/redundant_locals.rs:55:9
8383
|
8484
LL | let x = x;
8585
| ^
8686

8787
error: redundant redefinition of a binding `a`
88-
--> $DIR/redundant_locals.rs:61:5
88+
--> $DIR/redundant_locals.rs:62:5
8989
|
9090
LL | let a = a;
9191
| ^^^^^^^^^^
9292
|
9393
help: `a` is initially defined here
94-
--> $DIR/redundant_locals.rs:59:9
94+
--> $DIR/redundant_locals.rs:60:9
9595
|
9696
LL | let a = 1;
9797
| ^
9898

9999
error: redundant redefinition of a binding `b`
100-
--> $DIR/redundant_locals.rs:62:5
100+
--> $DIR/redundant_locals.rs:63:5
101101
|
102102
LL | let b = b;
103103
| ^^^^^^^^^^
104104
|
105105
help: `b` is initially defined here
106-
--> $DIR/redundant_locals.rs:60:9
106+
--> $DIR/redundant_locals.rs:61:9
107107
|
108108
LL | let b = 2;
109109
| ^
110110

111111
error: redundant redefinition of a binding `x`
112-
--> $DIR/redundant_locals.rs:68:9
112+
--> $DIR/redundant_locals.rs:69:9
113113
|
114114
LL | let x = x;
115115
| ^^^^^^^^^^
116116
|
117117
help: `x` is initially defined here
118-
--> $DIR/redundant_locals.rs:67:13
118+
--> $DIR/redundant_locals.rs:68:13
119119
|
120120
LL | let x = 1;
121121
| ^
122122

123123
error: redundant redefinition of a binding `x`
124-
--> $DIR/redundant_locals.rs:75:9
124+
--> $DIR/redundant_locals.rs:76:9
125125
|
126126
LL | let x = x;
127127
| ^^^^^^^^^^
128128
|
129129
help: `x` is initially defined here
130-
--> $DIR/redundant_locals.rs:74:13
130+
--> $DIR/redundant_locals.rs:75:13
131131
|
132132
LL | let x = 1;
133133
| ^
134134

135135
error: redundant redefinition of a binding `x`
136-
--> $DIR/redundant_locals.rs:78:9
136+
--> $DIR/redundant_locals.rs:79:9
137137
|
138138
LL | let x = x;
139139
| ^^^^^^^^^^
140140
|
141141
help: `x` is initially defined here
142-
--> $DIR/redundant_locals.rs:77:6
142+
--> $DIR/redundant_locals.rs:78:6
143143
|
144144
LL | |x: i32| {
145145
| ^
146146

147147
error: redundant redefinition of a binding `x`
148-
--> $DIR/redundant_locals.rs:97:9
148+
--> $DIR/redundant_locals.rs:98:9
149149
|
150150
LL | let x = x;
151151
| ^^^^^^^^^^
152152
|
153153
help: `x` is initially defined here
154-
--> $DIR/redundant_locals.rs:94:9
154+
--> $DIR/redundant_locals.rs:95:9
155155
|
156156
LL | let x = 1;
157157
| ^
158158

159159
error: redundant redefinition of a binding `a`
160-
--> $DIR/redundant_locals.rs:152:5
160+
--> $DIR/redundant_locals.rs:153:5
161161
|
162162
LL | let a = a;
163163
| ^^^^^^^^^^
164164
|
165165
help: `a` is initially defined here
166-
--> $DIR/redundant_locals.rs:150:9
166+
--> $DIR/redundant_locals.rs:151:9
167167
|
168168
LL | let a = WithoutDrop(1);
169169
| ^

0 commit comments

Comments
 (0)