Skip to content

Commit fdf819d

Browse files
borsbors
committed
Auto merge of #12227 - y21:issue12225, r=Manishearth
[`redundant_locals`]: take by-value closure captures into account Fixes #12225 The same problem in the linked issue can happen to regular closures too, and conveniently async blocks are closures in the HIR so fixing closures will fix async blocks as well. changelog: [`redundant_locals`]: avoid linting when redefined variable is captured by-value
2 parents 005b6c2 + 6807977 commit fdf819d

File tree

3 files changed

+100
-28
lines changed

3 files changed

+100
-28
lines changed

‎clippy_lints/src/redundant_locals.rs

+26
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ use clippy_utils::ty::needs_ordered_drop;
44
use rustc_ast::Mutability;
55
use rustc_hir::def::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_by_value_closure_capture(cx, local.hir_id, binding_id)
7275
{
7376
span_lint_and_help(
7477
cx,
@@ -82,6 +85,29 @@ 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_by_value_closure_capture(cx: &LateContext<'_>, redefinition: HirId, root_variable: HirId) -> bool {
102+
let closure_def_id = cx.tcx.hir().enclosing_body_owner(redefinition);
103+
104+
cx.tcx.is_closure_or_coroutine(closure_def_id.to_def_id())
105+
&& cx.tcx.closure_captures(closure_def_id).iter().any(|c| {
106+
matches!(c.info.capture_kind, UpvarCapture::ByValue)
107+
&& matches!(c.place.base, PlaceBase::Upvar(upvar) if upvar.var_path.hir_id == root_variable)
108+
})
109+
}
110+
85111
/// Find the annotation of a binding introduced by a pattern, or `None` if it's not introduced.
86112
fn find_binding(pat: &Pat<'_>, name: Ident) -> Option<BindingAnnotation> {
87113
let mut ret = None;

‎tests/ui/redundant_locals.rs

+46
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, coroutines)]
45

56
extern crate proc_macros;
67
use proc_macros::{external, with_span};
@@ -163,3 +164,48 @@ 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+
let v5 = String::new();
176+
let v6 = String::new();
177+
178+
assert_static(|| {
179+
let v1 = v1;
180+
dbg!(&v1);
181+
});
182+
assert_static(async {
183+
let v2 = v2;
184+
dbg!(&v2);
185+
});
186+
assert_static(|| async {
187+
let v3 = v3;
188+
dbg!(&v3);
189+
});
190+
assert_static(async || {
191+
let v4 = v4;
192+
dbg!(&v4);
193+
});
194+
assert_static(static || {
195+
let v5 = v5;
196+
yield;
197+
});
198+
assert_static(|| {
199+
let v6 = v6;
200+
yield;
201+
});
202+
203+
fn foo(a: &str, b: &str) {}
204+
205+
let do_not_move = String::new();
206+
let things_to_move = vec!["a".to_string(), "b".to_string()];
207+
let futures = things_to_move.into_iter().map(|move_me| async {
208+
let move_me = move_me;
209+
foo(&do_not_move, &move_me)
210+
});
211+
}

‎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)