Skip to content

Commit 9d6ecc7

Browse files
author
Michael Wright
committed
Fix boxed_local suggestion
Don't warn about an argument that is moved into a closure. ExprUseVisitor doesn't walk into nested bodies so use a new visitor that collects the variables that are moved into closures. Fixes #3739
1 parent 027dde9 commit 9d6ecc7

File tree

3 files changed

+66
-4
lines changed

3 files changed

+66
-4
lines changed

clippy_lints/src/escape.rs

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
use crate::utils::span_lint;
22
use rustc::hir::intravisit as visit;
3+
use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
34
use rustc::hir::*;
45
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
56
use rustc::middle::expr_use_visitor::*;
67
use rustc::middle::mem_categorization::{cmt_, Categorization};
78
use rustc::ty::layout::LayoutOf;
8-
use rustc::ty::{self, Ty};
9+
use rustc::ty::{self, Ty, UpvarCapture};
910
use rustc::util::nodemap::NodeSet;
1011
use rustc::{declare_tool_lint, lint_array};
1112
use syntax::ast::NodeId;
@@ -88,11 +89,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
8889
let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
8990
ExprUseVisitor::new(&mut v, cx.tcx, cx.param_env, region_scope_tree, cx.tables, None).consume_body(body);
9091

91-
for node in v.set {
92+
let mut capture_visitor = CaptureVisitor {
93+
cx,
94+
moved: NodeSet::default(),
95+
};
96+
capture_visitor.visit_body(body);
97+
98+
for node in v.set.difference(&capture_visitor.moved) {
9299
span_lint(
93100
cx,
94101
BOXED_LOCAL,
95-
cx.tcx.hir().span(node),
102+
cx.tcx.hir().span(*node),
96103
"local variable doesn't need to be boxed here",
97104
);
98105
}
@@ -192,3 +199,32 @@ impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> {
192199
}
193200
}
194201
}
202+
203+
struct CaptureVisitor<'a, 'tcx: 'a> {
204+
cx: &'a LateContext<'a, 'tcx>,
205+
moved: NodeSet,
206+
}
207+
208+
impl<'a, 'tcx> Visitor<'tcx> for CaptureVisitor<'a, 'tcx> {
209+
fn visit_expr(&mut self, expr: &'tcx Expr) {
210+
if let ExprKind::Closure(..) = expr.node {
211+
if let ty::Closure(def_id, _) = &self.cx.tables.expr_ty(expr).sty {
212+
if let Some(upvar_list) = &self.cx.tables.upvar_list.get(&def_id) {
213+
for upvar_id in upvar_list.iter() {
214+
if let UpvarCapture::ByValue = self.cx.tables.upvar_capture(*upvar_id) {
215+
let hir_id = upvar_id.var_path.hir_id;
216+
let id = &self.cx.tcx.hir().hir_to_node_id(hir_id);
217+
self.moved.insert(*id);
218+
}
219+
}
220+
}
221+
}
222+
} else {
223+
walk_expr(self, expr);
224+
}
225+
}
226+
227+
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
228+
NestedVisitorMap::None
229+
}
230+
}

tests/ui/escape_analysis.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,23 @@ trait MyTrait {
148148
impl<T> MyTrait for Box<T> {
149149
fn do_sth(self) {}
150150
}
151+
152+
// Issue #3739 - capture in closures
153+
mod issue_3739 {
154+
use super::A;
155+
156+
fn consume<T>(_: T) {}
157+
fn borrow<T>(_: &T) {}
158+
159+
fn closure_consume(x: Box<A>) {
160+
let _ = move || {
161+
consume(x);
162+
};
163+
}
164+
165+
fn closure_borrow(x: Box<A>) {
166+
let _ = || {
167+
borrow(&x);
168+
};
169+
}
170+
}

tests/ui/escape_analysis.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,11 @@ error: local variable doesn't need to be boxed here
1212
LL | pub fn new(_needs_name: Box<PeekableSeekable<&()>>) -> () {}
1313
| ^^^^^^^^^^^
1414

15-
error: aborting due to 2 previous errors
15+
error: local variable doesn't need to be boxed here
16+
--> $DIR/escape_analysis.rs:165:23
17+
|
18+
LL | fn closure_borrow(x: Box<A>) {
19+
| ^
20+
21+
error: aborting due to 3 previous errors
1622

0 commit comments

Comments
 (0)