Skip to content

Commit

Permalink
Auto merge of #8220 - Jarcho:manual_swap_8154, r=camsteffen
Browse files Browse the repository at this point in the history
Consider auto-deref when linting `manual_swap`

fixes #8154

changelog: Don't lint `manual_swap` when a field access involves auto-deref
  • Loading branch information
bors committed Jan 4, 2022
2 parents 3ea7784 + a7097b8 commit 0e28e38
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 1 deletion.
21 changes: 21 additions & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,19 @@ fn projection_stack<'a, 'hir>(mut e: &'a Expr<'hir>) -> (Vec<&'a Expr<'hir>>, &'
(result, root)
}

/// Gets the mutability of the custom deref adjustment, if any.
pub fn expr_custom_deref_adjustment(cx: &LateContext<'_>, e: &Expr<'_>) -> Option<Mutability> {
cx.typeck_results()
.expr_adjustments(e)
.iter()
.find_map(|a| match a.kind {
Adjust::Deref(Some(d)) => Some(Some(d.mutbl)),
Adjust::Deref(None) => None,
_ => Some(None),
})
.and_then(|x| x)
}

/// Checks if two expressions can be mutably borrowed simultaneously
/// and they aren't dependent on borrowing same thing twice
pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -> bool {
Expand All @@ -629,7 +642,15 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -
if !eq_expr_value(cx, r1, r2) {
return true;
}
if expr_custom_deref_adjustment(cx, r1).is_some() || expr_custom_deref_adjustment(cx, r2).is_some() {
return false;
}

for (x1, x2) in s1.iter().zip(s2.iter()) {
if expr_custom_deref_adjustment(cx, x1).is_some() || expr_custom_deref_adjustment(cx, x2).is_some() {
return false;
}

match (&x1.kind, &x2.kind) {
(ExprKind::Field(_, i1), ExprKind::Field(_, i2)) => {
if i1 != i2 {
Expand Down
33 changes: 33 additions & 0 deletions tests/ui/swap.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,36 @@ fn main() {

; std::mem::swap(&mut c.0, &mut a);
}

fn issue_8154() {
struct S1 {
x: i32,
y: i32,
}
struct S2(S1);
struct S3<'a, 'b>(&'a mut &'b mut S1);

impl std::ops::Deref for S2 {
type Target = S1;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl std::ops::DerefMut for S2 {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

// Don't lint. `s.0` is mutably borrowed by `s.x` and `s.y` via the deref impl.
let mut s = S2(S1 { x: 0, y: 0 });
let t = s.x;
s.x = s.y;
s.y = t;

// Accessing through a mutable reference is fine
let mut s = S1 { x: 0, y: 0 };
let mut s = &mut s;
let s = S3(&mut s);
std::mem::swap(&mut s.0.x, &mut s.0.y);
}
35 changes: 35 additions & 0 deletions tests/ui/swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,38 @@ fn main() {
c.0 = a;
a = t;
}

fn issue_8154() {
struct S1 {
x: i32,
y: i32,
}
struct S2(S1);
struct S3<'a, 'b>(&'a mut &'b mut S1);

impl std::ops::Deref for S2 {
type Target = S1;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl std::ops::DerefMut for S2 {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

// Don't lint. `s.0` is mutably borrowed by `s.x` and `s.y` via the deref impl.
let mut s = S2(S1 { x: 0, y: 0 });
let t = s.x;
s.x = s.y;
s.y = t;

// Accessing through a mutable reference is fine
let mut s = S1 { x: 0, y: 0 };
let mut s = &mut s;
let s = S3(&mut s);
let t = s.0.x;
s.0.x = s.0.y;
s.0.y = t;
}
12 changes: 11 additions & 1 deletion tests/ui/swap.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -108,5 +108,15 @@ LL | | a = c.0;
|
= note: or maybe you should use `std::mem::replace`?

error: aborting due to 12 previous errors
error: this looks like you are swapping `s.0.x` and `s.0.y` manually
--> $DIR/swap.rs:178:5
|
LL | / let t = s.0.x;
LL | | s.0.x = s.0.y;
LL | | s.0.y = t;
| |_____________^ help: try: `std::mem::swap(&mut s.0.x, &mut s.0.y)`
|
= note: or maybe you should use `std::mem::replace`?

error: aborting due to 13 previous errors

0 comments on commit 0e28e38

Please sign in to comment.