Skip to content

Commit

Permalink
Don't emit needless_pass_by_ref_mut if the variable is used in an u…
Browse files Browse the repository at this point in the history
…nsafe block or function
  • Loading branch information
GuillaumeGomez committed Oct 6, 2023
1 parent 9554e47 commit 8cefb89
Showing 1 changed file with 38 additions and 2 deletions.
40 changes: 38 additions & 2 deletions clippy_lints/src/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_qpath, FnKind, Visitor};
use rustc_hir::{
Body, Closure, Expr, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, PatKind, QPath,
BlockCheckMode, Body, 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::{InferCtxt, TyCtxtInferExt};
Expand Down Expand Up @@ -139,13 +140,23 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
let hir_id = cx.tcx.hir().local_def_id_to_hir_id(fn_def_id);
let is_async = match kind {
FnKind::ItemFn(.., header) => {
if header.is_unsafe() {
// We don't check unsafe functions.
return;
}
let attrs = cx.tcx.hir().attrs(hir_id);
if header.abi != Abi::Rust || requires_exact_signature(attrs) {
return;
}
header.is_async()
},
FnKind::Method(.., sig) => sig.header.is_async(),
FnKind::Method(.., sig) => {
if sig.header.is_unsafe() {
// We don't check unsafe functions.
return;
}
sig.header.is_async()
},
FnKind::Closure => return,
};

Expand Down Expand Up @@ -304,6 +315,23 @@ impl<'tcx> MutablyUsedVariablesCtxt<'tcx> {
}
self.aliases.insert(alias, target);
}

// The goal here is to find if the current scope is unsafe or not. It stops when it finds
// a function or an unsafe block.
fn is_in_unsafe_block(&self, mut item: HirId) -> bool {
let hir = self.tcx.hir();
while let Some(parent) = hir.get_enclosing_scope(item) {
if let Some(fn_sig) = hir.fn_sig_by_hir_id(parent) {
return fn_sig.header.is_unsafe();
} else if let Some(Node::Block(block)) = hir.find(parent) {
if matches!(block.rules, BlockCheckMode::UnsafeBlock(_)) {
return true;
}
}
item = parent;
}
false
}
}

impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt<'tcx> {
Expand All @@ -327,6 +355,10 @@ impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt<'tcx> {
&& matches!(base_ty.ref_mutability(), Some(Mutability::Mut))
{
self.add_mutably_used_var(*vid);
} else if self.is_in_unsafe_block(*vid) {
// If we are in an unsafe block, any operation on this variable must not be warned
// upon!
self.add_mutably_used_var(*vid);
}
self.prev_bind = None;
self.prev_move_to_closure.remove(vid);
Expand Down Expand Up @@ -355,6 +387,10 @@ impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt<'tcx> {
|| (borrow == ty::BorrowKind::UniqueImmBorrow && base_ty.ref_mutability() == Some(Mutability::Mut))
{
self.add_mutably_used_var(*vid);
} else if self.is_in_unsafe_block(*vid) {
// If we are in an unsafe block, any operation on this variable must not be warned
// upon!
self.add_mutably_used_var(*vid);
}
} else if borrow == ty::ImmBorrow {
// If there is an `async block`, it'll contain a call to a closure which we need to
Expand Down

0 comments on commit 8cefb89

Please sign in to comment.