[redundant_locals]: take by-value closure captures into account

This commit is contained in:
y21 2024-02-04 20:12:15 +01:00
parent 9fb41079ca
commit 7c3908f86c
3 changed files with 98 additions and 34 deletions

View file

@ -2,10 +2,12 @@ use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_from_proc_macro; use clippy_utils::is_from_proc_macro;
use clippy_utils::ty::needs_ordered_drop; use clippy_utils::ty::needs_ordered_drop;
use rustc_ast::Mutability; use rustc_ast::Mutability;
use rustc_hir::def::Res; use rustc_hir::def::{DefKind, Res};
use rustc_hir::{BindingAnnotation, ByRef, ExprKind, HirId, Local, Node, Pat, PatKind, QPath}; use rustc_hir::{BindingAnnotation, ByRef, ExprKind, HirId, Local, Node, Pat, PatKind, QPath};
use rustc_hir_typeck::expr_use_visitor::PlaceBase;
use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro; use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::UpvarCapture;
use rustc_session::declare_lint_pass; use rustc_session::declare_lint_pass;
use rustc_span::symbol::Ident; use rustc_span::symbol::Ident;
use rustc_span::DesugaringKind; use rustc_span::DesugaringKind;
@ -69,6 +71,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantLocals {
// the local is user-controlled // the local is user-controlled
&& !in_external_macro(cx.sess(), local.span) && !in_external_macro(cx.sess(), local.span)
&& !is_from_proc_macro(cx, expr) && !is_from_proc_macro(cx, expr)
&& !is_closure_capture(cx, local.hir_id, binding_id)
{ {
span_lint_and_help( span_lint_and_help(
cx, cx,
@ -82,6 +85,31 @@ impl<'tcx> LateLintPass<'tcx> for RedundantLocals {
} }
} }
/// Checks if the enclosing body is a closure and if the given local is captured by value.
///
/// In those cases, the redefinition may be necessary to force a move:
/// ```
/// fn assert_static<T: 'static>(_: T) {}
///
/// let v = String::new();
/// let closure = || {
/// let v = v; // <- removing this redefinition makes `closure` no longer `'static`
/// dbg!(&v);
/// };
/// assert_static(closure);
/// ```
fn is_closure_capture(cx: &LateContext<'_>, redefinition: HirId, root_variable: HirId) -> bool {
let body = cx.tcx.hir().enclosing_body_owner(redefinition);
if let DefKind::Closure = cx.tcx.def_kind(body) {
cx.tcx.closure_captures(body).iter().any(|c| {
matches!(c.info.capture_kind, UpvarCapture::ByValue)
&& matches!(c.place.base, PlaceBase::Upvar(upvar) if upvar.var_path.hir_id == root_variable)
})
} else {
false
}
}
/// Find the annotation of a binding introduced by a pattern, or `None` if it's not introduced. /// Find the annotation of a binding introduced by a pattern, or `None` if it's not introduced.
fn find_binding(pat: &Pat<'_>, name: Ident) -> Option<BindingAnnotation> { fn find_binding(pat: &Pat<'_>, name: Ident) -> Option<BindingAnnotation> {
let mut ret = None; let mut ret = None;

View file

@ -1,6 +1,7 @@
//@aux-build:proc_macros.rs //@aux-build:proc_macros.rs
#![allow(unused, clippy::no_effect, clippy::needless_pass_by_ref_mut)] #![allow(unused, clippy::no_effect, clippy::needless_pass_by_ref_mut)]
#![warn(clippy::redundant_locals)] #![warn(clippy::redundant_locals)]
#![feature(async_closure)]
extern crate proc_macros; extern crate proc_macros;
use proc_macros::{external, with_span}; use proc_macros::{external, with_span};
@ -163,3 +164,38 @@ fn drop_compose() {
let b = ComposeDrop { d: WithDrop(1) }; let b = ComposeDrop { d: WithDrop(1) };
let a = a; let a = a;
} }
fn issue12225() {
fn assert_static<T: 'static>(_: T) {}
let v1 = String::new();
let v2 = String::new();
let v3 = String::new();
let v4 = String::new();
assert_static(|| {
let v1 = v1;
dbg!(&v1);
});
assert_static(async {
let v2 = v2;
dbg!(&v2);
});
assert_static(|| async {
let v3 = v3;
dbg!(&v3);
});
assert_static(async || {
let v4 = v4;
dbg!(&v4);
});
fn foo(a: &str, b: &str) {}
let do_not_move = String::new();
let things_to_move = vec!["a".to_string(), "b".to_string()];
let futures = things_to_move.into_iter().map(|move_me| async {
let move_me = move_me;
foo(&do_not_move, &move_me)
});
}

View file

@ -1,11 +1,11 @@
error: redundant redefinition of a binding `x` error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:12:5 --> $DIR/redundant_locals.rs:13:5
| |
LL | let x = x; LL | let x = x;
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
help: `x` is initially defined here help: `x` is initially defined here
--> $DIR/redundant_locals.rs:11:9 --> $DIR/redundant_locals.rs:12:9
| |
LL | let x = 1; LL | let x = 1;
| ^ | ^
@ -13,41 +13,29 @@ LL | let x = 1;
= help: to override `-D warnings` add `#[allow(clippy::redundant_locals)]` = help: to override `-D warnings` add `#[allow(clippy::redundant_locals)]`
error: redundant redefinition of a binding `x` error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:17:5 --> $DIR/redundant_locals.rs:18:5
| |
LL | let mut x = x; LL | let mut x = x;
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
| |
help: `x` is initially defined here help: `x` is initially defined here
--> $DIR/redundant_locals.rs:16:9 --> $DIR/redundant_locals.rs:17:9
| |
LL | let mut x = 1; LL | let mut x = 1;
| ^^^^^ | ^^^^^
error: redundant redefinition of a binding `x` error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:47:5 --> $DIR/redundant_locals.rs:48:5
| |
LL | let x = x; LL | let x = x;
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
help: `x` is initially defined here help: `x` is initially defined here
--> $DIR/redundant_locals.rs:46:14 --> $DIR/redundant_locals.rs:47:14
| |
LL | fn parameter(x: i32) { LL | fn parameter(x: i32) {
| ^ | ^
error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:52:5
|
LL | let x = x;
| ^^^^^^^^^^
|
help: `x` is initially defined here
--> $DIR/redundant_locals.rs:51:9
|
LL | let x = 1;
| ^
error: redundant redefinition of a binding `x` error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:53:5 --> $DIR/redundant_locals.rs:53:5
| |
@ -57,7 +45,7 @@ LL | let x = x;
help: `x` is initially defined here help: `x` is initially defined here
--> $DIR/redundant_locals.rs:52:9 --> $DIR/redundant_locals.rs:52:9
| |
LL | let x = x; LL | let x = 1;
| ^ | ^
error: redundant redefinition of a binding `x` error: redundant redefinition of a binding `x`
@ -84,86 +72,98 @@ help: `x` is initially defined here
LL | let x = x; LL | let x = x;
| ^ | ^
error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:56:5
|
LL | let x = x;
| ^^^^^^^^^^
|
help: `x` is initially defined here
--> $DIR/redundant_locals.rs:55:9
|
LL | let x = x;
| ^
error: redundant redefinition of a binding `a` error: redundant redefinition of a binding `a`
--> $DIR/redundant_locals.rs:61:5 --> $DIR/redundant_locals.rs:62:5
| |
LL | let a = a; LL | let a = a;
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
help: `a` is initially defined here help: `a` is initially defined here
--> $DIR/redundant_locals.rs:59:9 --> $DIR/redundant_locals.rs:60:9
| |
LL | let a = 1; LL | let a = 1;
| ^ | ^
error: redundant redefinition of a binding `b` error: redundant redefinition of a binding `b`
--> $DIR/redundant_locals.rs:62:5 --> $DIR/redundant_locals.rs:63:5
| |
LL | let b = b; LL | let b = b;
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
help: `b` is initially defined here help: `b` is initially defined here
--> $DIR/redundant_locals.rs:60:9 --> $DIR/redundant_locals.rs:61:9
| |
LL | let b = 2; LL | let b = 2;
| ^ | ^
error: redundant redefinition of a binding `x` error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:68:9 --> $DIR/redundant_locals.rs:69:9
| |
LL | let x = x; LL | let x = x;
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
help: `x` is initially defined here help: `x` is initially defined here
--> $DIR/redundant_locals.rs:67:13 --> $DIR/redundant_locals.rs:68:13
| |
LL | let x = 1; LL | let x = 1;
| ^ | ^
error: redundant redefinition of a binding `x` error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:75:9 --> $DIR/redundant_locals.rs:76:9
| |
LL | let x = x; LL | let x = x;
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
help: `x` is initially defined here help: `x` is initially defined here
--> $DIR/redundant_locals.rs:74:13 --> $DIR/redundant_locals.rs:75:13
| |
LL | let x = 1; LL | let x = 1;
| ^ | ^
error: redundant redefinition of a binding `x` error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:78:9 --> $DIR/redundant_locals.rs:79:9
| |
LL | let x = x; LL | let x = x;
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
help: `x` is initially defined here help: `x` is initially defined here
--> $DIR/redundant_locals.rs:77:6 --> $DIR/redundant_locals.rs:78:6
| |
LL | |x: i32| { LL | |x: i32| {
| ^ | ^
error: redundant redefinition of a binding `x` error: redundant redefinition of a binding `x`
--> $DIR/redundant_locals.rs:97:9 --> $DIR/redundant_locals.rs:98:9
| |
LL | let x = x; LL | let x = x;
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
help: `x` is initially defined here help: `x` is initially defined here
--> $DIR/redundant_locals.rs:94:9 --> $DIR/redundant_locals.rs:95:9
| |
LL | let x = 1; LL | let x = 1;
| ^ | ^
error: redundant redefinition of a binding `a` error: redundant redefinition of a binding `a`
--> $DIR/redundant_locals.rs:152:5 --> $DIR/redundant_locals.rs:153:5
| |
LL | let a = a; LL | let a = a;
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
help: `a` is initially defined here help: `a` is initially defined here
--> $DIR/redundant_locals.rs:150:9 --> $DIR/redundant_locals.rs:151:9
| |
LL | let a = WithoutDrop(1); LL | let a = WithoutDrop(1);
| ^ | ^