Skip to content

Commit

Permalink
Auto merge of rust-lang#12693 - GuillaumeGomez:run-on-self-needless_p…
Browse files Browse the repository at this point in the history 
…ass_by_ref_mut, r=Manishearth

Emit the `needless_pass_by_ref_mut` lint on `self` arguments as well

Fixes rust-lang/rust-clippy#12589.
Fixes rust-lang/rust-clippy#9591.

The first commit fixes a bug I uncovered while working on this: sometimes, the mutable borrow "event" happens before the alias one, which makes some argument detected as not used mutably even if they are. The fix was simply to fill the map with the aliases afterwards.

The second commit removes the restriction to not run `self` argument for the `needless_pass_by_ref_mut` lint.

changelog: emit the `needless_pass_by_ref_mut` lint on `self` arguments as well

r? `@Manishearth`
  • Loading branch information
@bors
bors committed Apr 18, 2024
2 parents ca3b393 + 0999867 commit 2795a60
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 54 deletions.
26 changes: 19 additions & 7 deletions clippy_lints/src/needless_pass_by_ref_mut.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ fn should_skip<'tcx>(
}

if is_self(arg) {
return true;
// Interestingly enough, `self` arguments make `is_from_proc_macro` return `true`, hence why
// we return early here.
return false;
}

if let PatKind::Binding(.., name, _) = arg.pat.kind {
Expand Down Expand Up @@ -185,7 +187,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
}
// Collect variables mutably used and spans which will need dereferencings from the
// function body.
let MutablyUsedVariablesCtxt { mutably_used_vars, .. } = {
let mutably_used_vars = {
let mut ctx = MutablyUsedVariablesCtxt {
mutably_used_vars: HirIdSet::default(),
prev_bind: None,
Expand Down Expand Up @@ -217,7 +219,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
check_closures(&mut ctx, cx, &infcx, &mut checked_closures, async_closures);
}
}
ctx
ctx.generate_mutably_used_ids_from_aliases()
};
for ((&input, &_), arg) in it {
// Only take `&mut` arguments.
Expand Down Expand Up @@ -309,12 +311,22 @@ struct MutablyUsedVariablesCtxt<'tcx> {
}

impl<'tcx> MutablyUsedVariablesCtxt<'tcx> {
fn add_mutably_used_var(&mut self, mut used_id: HirId) {
while let Some(id) = self.aliases.get(&used_id) {
fn add_mutably_used_var(&mut self, used_id: HirId) {
self.mutably_used_vars.insert(used_id);
}

// Because the alias may come after the mutable use of a variable, we need to fill the map at
// the end.
fn generate_mutably_used_ids_from_aliases(mut self) -> HirIdSet {
let all_ids = self.mutably_used_vars.iter().copied().collect::<Vec<_>>();
for mut used_id in all_ids {
while let Some(id) = self.aliases.get(&used_id) {
self.mutably_used_vars.insert(used_id);
used_id = *id;
}
self.mutably_used_vars.insert(used_id);
used_id = *id;
}
self.mutably_used_vars.insert(used_id);
self.mutably_used_vars
}

fn would_be_alias_cycle(&self, alias: HirId, mut target: HirId) -> bool {
Expand Down
42 changes: 36 additions & 6 deletions tests/ui/needless_pass_by_ref_mut.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,13 @@ fn non_mut_ref(_: &Vec<u32>) {}
struct Bar;

impl Bar {
// Should not warn on `&mut self`.
fn bar(&mut self) {}
//~^ ERROR: this argument is a mutable reference, but not used mutably

fn mushroom(&self, vec: &mut Vec<i32>) -> usize {
//~^ ERROR: this argument is a mutable reference, but not used mutably
vec.len()
}

fn badger(&mut self, vec: &mut Vec<i32>) -> usize {
//~^ ERROR: this argument is a mutable reference, but not used mutably
vec.len()
}
}

trait Babar {
Expand Down Expand Up @@ -307,6 +302,41 @@ fn filter_copy<T: Copy>(predicate: &mut impl FnMut(T) -> bool) -> impl FnMut(&T)
move |&item| predicate(item)
}

trait MutSelfTrait {
// Should not warn since it's a trait method.
fn mut_self(&mut self);
}

struct MutSelf {
a: u32,
}

impl MutSelf {
fn bar(&mut self) {}
//~^ ERROR: this argument is a mutable reference, but not used mutably
async fn foo(&mut self, u: &mut i32, v: &mut u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
//~| ERROR: this argument is a mutable reference, but not used mutably
async {
*u += 1;
}
.await;
}
async fn foo2(&mut self, u: &mut i32, v: &mut u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
async {
self.a += 1;
*u += 1;
}
.await;
}
}

impl MutSelfTrait for MutSelf {
// Should not warn since it's a trait method.
fn mut_self(&mut self) {}
}

// `is_from_proc_macro` stress tests
fn _empty_tup(x: &mut (())) {}
fn _single_tup(x: &mut ((i32,))) {}
Expand Down
98 changes: 57 additions & 41 deletions tests/ui/needless_pass_by_ref_mut.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,190 +14,206 @@ LL | fn foo6(s: &mut Vec<u32>) {
| ^^^^^^^^^^^^^ help: consider changing to: `&Vec<u32>`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:50:29
--> tests/ui/needless_pass_by_ref_mut.rs:47:12
|
LL | fn mushroom(&self, vec: &mut Vec<i32>) -> usize {
| ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`
LL | fn bar(&mut self) {}
| ^^^^^^^^^ help: consider changing to: `&self`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:55:31
--> tests/ui/needless_pass_by_ref_mut.rs:50:29
|
LL | fn badger(&mut self, vec: &mut Vec<i32>) -> usize {
| ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`
LL | fn mushroom(&self, vec: &mut Vec<i32>) -> usize {
| ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:132:16
--> tests/ui/needless_pass_by_ref_mut.rs:127:16
|
LL | async fn a1(x: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:136:16
--> tests/ui/needless_pass_by_ref_mut.rs:131:16
|
LL | async fn a2(x: &mut i32, y: String) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:140:16
--> tests/ui/needless_pass_by_ref_mut.rs:135:16
|
LL | async fn a3(x: &mut i32, y: String, z: String) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:144:16
--> tests/ui/needless_pass_by_ref_mut.rs:139:16
|
LL | async fn a4(x: &mut i32, y: i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:148:24
--> tests/ui/needless_pass_by_ref_mut.rs:143:24
|
LL | async fn a5(x: i32, y: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:152:24
--> tests/ui/needless_pass_by_ref_mut.rs:147:24
|
LL | async fn a6(x: i32, y: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:156:32
--> tests/ui/needless_pass_by_ref_mut.rs:151:32
|
LL | async fn a7(x: i32, y: i32, z: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:160:24
--> tests/ui/needless_pass_by_ref_mut.rs:155:24
|
LL | async fn a8(x: i32, a: &mut i32, y: i32, z: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:160:45
--> tests/ui/needless_pass_by_ref_mut.rs:155:45
|
LL | async fn a8(x: i32, a: &mut i32, y: i32, z: &mut i32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:194:16
--> tests/ui/needless_pass_by_ref_mut.rs:189:16
|
LL | fn cfg_warn(s: &mut u32) {}
| ^^^^^^^^ help: consider changing to: `&u32`
|
= note: this is cfg-gated and may require further changes

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:200:20
--> tests/ui/needless_pass_by_ref_mut.rs:195:20
|
LL | fn cfg_warn(s: &mut u32) {}
| ^^^^^^^^ help: consider changing to: `&u32`
|
= note: this is cfg-gated and may require further changes

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:214:39
--> tests/ui/needless_pass_by_ref_mut.rs:209:39
|
LL | async fn inner_async2(x: &mut i32, y: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&u32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:222:26
--> tests/ui/needless_pass_by_ref_mut.rs:217:26
|
LL | async fn inner_async3(x: &mut i32, y: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:241:34
--> tests/ui/needless_pass_by_ref_mut.rs:236:34
|
LL | pub async fn call_in_closure1(n: &mut str) {
| ^^^^^^^^ help: consider changing to: `&str`
|
= warning: changing this function will impact semver compatibility

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:253: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
--> tests/ui/needless_pass_by_ref_mut.rs:260:20
--> tests/ui/needless_pass_by_ref_mut.rs:255: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
--> tests/ui/needless_pass_by_ref_mut.rs:271:26
--> tests/ui/needless_pass_by_ref_mut.rs:266:26
|
LL | pub async fn closure4(n: &mut 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
--> tests/ui/needless_pass_by_ref_mut.rs:311:18
--> tests/ui/needless_pass_by_ref_mut.rs:315:12
|
LL | fn bar(&mut self) {}
| ^^^^^^^^^ help: consider changing to: `&self`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:317:18
|
LL | async fn foo(&mut self, u: &mut i32, v: &mut u32) {
| ^^^^^^^^^ help: consider changing to: `&self`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:317:45
|
LL | async fn foo(&mut self, u: &mut i32, v: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&u32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:325:46
|
LL | async fn foo2(&mut self, u: &mut i32, v: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&u32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:341:18
|
LL | fn _empty_tup(x: &mut (())) {}
| ^^^^^^^^^ help: consider changing to: `&()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:312:19
--> tests/ui/needless_pass_by_ref_mut.rs:342:19
|
LL | fn _single_tup(x: &mut ((i32,))) {}
| ^^^^^^^^^^^^^ help: consider changing to: `&(i32,)`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:313:18
--> tests/ui/needless_pass_by_ref_mut.rs:343:18
|
LL | fn _multi_tup(x: &mut ((i32, u32))) {}
| ^^^^^^^^^^^^^^^^^ help: consider changing to: `&(i32, u32)`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:314:11
--> tests/ui/needless_pass_by_ref_mut.rs:344:11
|
LL | fn _fn(x: &mut (fn())) {}
| ^^^^^^^^^^^ help: consider changing to: `&fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:316:23
--> tests/ui/needless_pass_by_ref_mut.rs:346:23
|
LL | fn _extern_rust_fn(x: &mut extern "Rust" fn()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "Rust" fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:317:20
--> tests/ui/needless_pass_by_ref_mut.rs:347:20
|
LL | fn _extern_c_fn(x: &mut extern "C" fn()) {}
| ^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "C" fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:318:18
--> tests/ui/needless_pass_by_ref_mut.rs:348:18
|
LL | fn _unsafe_fn(x: &mut unsafe fn()) {}
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:319:25
--> tests/ui/needless_pass_by_ref_mut.rs:349:25
|
LL | fn _unsafe_extern_fn(x: &mut unsafe extern "C" fn()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:320:20
--> tests/ui/needless_pass_by_ref_mut.rs:350:20
|
LL | fn _fn_with_arg(x: &mut unsafe extern "C" fn(i32)) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn(i32)`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:321:20
--> tests/ui/needless_pass_by_ref_mut.rs:351:20
|
LL | fn _fn_with_ret(x: &mut unsafe extern "C" fn() -> (i32)) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn() -> (i32)`

error: aborting due to 31 previous errors
error: aborting due to 34 previous errors

24 changes: 24 additions & 0 deletions tests/ui/needless_pass_by_ref_mut2.fixed
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// If both `inner_async3` and `inner_async4` are present, aliases are declared after
// they're used in `inner_async4` for some reasons... This test ensures that no
// only `v` is marked as not used mutably in `inner_async4`.

#![allow(clippy::redundant_closure_call)]
#![warn(clippy::needless_pass_by_ref_mut)]

pub async fn inner_async3(x: &i32, y: &mut u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
async {
*y += 1;
}
.await;
}

pub async fn inner_async4(u: &mut i32, v: &u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
async {
*u += 1;
}
.await;
}

fn main() {}
Loading

0 comments on commit 2795a60

Please sign in to comment.