Skip to content

Commit

Permalink
Fix mutaby used async function argument in closure for `needless_pass…
Browse files Browse the repository at this point in the history
…_by_ref_mut`
  • Loading branch information
GuillaumeGomez committed Sep 13, 2023
1 parent b788add commit e3267b1
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 21 deletions.
85 changes: 66 additions & 19 deletions clippy_lints/src/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ use clippy_utils::source::snippet;
use clippy_utils::{get_parent_node, inherits_cfg, is_from_proc_macro, is_self};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_qpath, FnKind, Visitor};
use rustc_hir::intravisit::{walk_fn, walk_qpath, FnKind, Visitor};
use rustc_hir::{
Body, Closure, Expr, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, PatKind, QPath,
Body, BodyId, Closure, Expr, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node,
PatKind, QPath,
};
use rustc_hir_typeck::expr_use_visitor as euv;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::associated_body;
use rustc_middle::hir::nested_filter::OnlyBodies;
Expand Down Expand Up @@ -95,6 +96,30 @@ fn should_skip<'tcx>(
is_from_proc_macro(cx, &input)
}

fn check_closures<'tcx>(
ctx: &mut MutablyUsedVariablesCtxt<'tcx>,
cx: &LateContext<'tcx>,
infcx: &InferCtxt<'tcx>,
checked_closures: &mut FxHashSet<LocalDefId>,
closures: FxHashSet<LocalDefId>,
) {
let hir = cx.tcx.hir();
for closure in closures {
if !checked_closures.insert(closure) {
continue;
}
ctx.prev_bind = None;
ctx.prev_move_to_closure.clear();
if let Some(body) = hir
.find_by_def_id(closure)
.and_then(associated_body)
.map(|(_, body_id)| hir.body(body_id))
{
euv::ExprUseVisitor::new(ctx, infcx, closure, cx.param_env, cx.typeck_results()).consume_body(body);
}
}
}

impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
fn check_fn(
&mut self,
Expand Down Expand Up @@ -161,25 +186,20 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
euv::ExprUseVisitor::new(&mut ctx, &infcx, fn_def_id, cx.param_env, cx.typeck_results()).consume_body(body);
if is_async {
let mut checked_closures = FxHashSet::default();

// We retrieve all the closures declared in the async function because they will
// not be found by `euv::Delegate`.
let mut closures_retriever = ClosuresRetriever {
cx,
closures: FxHashSet::default(),
};
walk_fn(&mut closures_retriever, kind, decl, body.id(), fn_def_id);
check_closures(&mut ctx, cx, &infcx, &mut checked_closures, closures_retriever.closures);

while !ctx.async_closures.is_empty() {
let closures = ctx.async_closures.clone();
ctx.async_closures.clear();
let hir = cx.tcx.hir();
for closure in closures {
if !checked_closures.insert(closure) {
continue;
}
ctx.prev_bind = None;
ctx.prev_move_to_closure.clear();
if let Some(body) = hir
.find_by_def_id(closure)
.and_then(associated_body)
.map(|(_, body_id)| hir.body(body_id))
{
euv::ExprUseVisitor::new(&mut ctx, &infcx, closure, cx.param_env, cx.typeck_results())
.consume_body(body);
}
}
check_closures(&mut ctx, cx, &infcx, &mut checked_closures, closures);
}
}
ctx
Expand Down Expand Up @@ -439,3 +459,30 @@ impl<'tcx> Visitor<'tcx> for FnNeedsMutVisitor<'_, 'tcx> {
}
}
}

struct ClosuresRetriever<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
closures: FxHashSet<LocalDefId>,
}

impl<'a, 'tcx> Visitor<'tcx> for ClosuresRetriever<'a, 'tcx> {
type NestedFilter = OnlyBodies;

fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}

fn visit_fn(
&mut self,
kind: FnKind<'tcx>,
decl: &'tcx FnDecl<'tcx>,
body_id: BodyId,
_span: Span,
fn_def_id: LocalDefId,
) {
if matches!(kind, FnKind::Closure) {
self.closures.insert(fn_def_id);
}
walk_fn(self, kind, decl, body_id, fn_def_id);
}
}
28 changes: 27 additions & 1 deletion tests/ui/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(clippy::if_same_then_else, clippy::no_effect)]
#![allow(clippy::if_same_then_else, clippy::no_effect, clippy::redundant_closure_call)]
#![feature(lint_reasons)]
//@no-rustfix
use std::ptr::NonNull;
Expand Down Expand Up @@ -231,6 +231,32 @@ async fn async_vec2(b: &mut Vec<bool>) {
b.push(true);
}

// Should not warn.
pub async fn closure(n: &mut usize) -> impl '_ + FnMut() {
|| {
*n += 1;
}
}

// Should warn.
pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
//~^ ERROR: this argument is a mutable reference, but not used mutably
|| *n + 1
}

// Should not warn.
pub async fn closure3(n: &mut usize) {
(|| *n += 1)();
}

// Should warn.
pub async fn closure4(n: &mut usize) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
(|| {
let _x = *n + 1;
})();
}

fn main() {
let mut u = 0;
let mut v = vec![0];
Expand Down
26 changes: 25 additions & 1 deletion tests/ui/needless_pass_by_ref_mut.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -107,5 +107,29 @@ error: this argument is a mutable reference, but not used mutably
LL | async fn inner_async3(x: &mut i32, y: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: aborting due to 17 previous errors
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:235:25
|
LL | pub async fn closure(n: &mut usize) -> impl '_ + FnMut() {
| ^^^^^^^^^^ help: consider changing to: `&usize`
|
= warning: changing this function will impact semver compatibility

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:242:20
|
LL | pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
| ^^^^^^^^^^ help: consider changing to: `&usize`
|
= warning: changing this function will impact semver compatibility

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:253:26
|
LL | pub async fn closure4(n: &mut usize) {
| ^^^^^^^^^^ help: consider changing to: `&usize`
|
= warning: changing this function will impact semver compatibility

error: aborting due to 20 previous errors

0 comments on commit e3267b1

Please sign in to comment.