Skip to content

Commit 0e28e38

Browse files
committed
Auto merge of rust-lang#8220 - Jarcho:manual_swap_8154, r=camsteffen
Consider auto-deref when linting `manual_swap` fixes rust-lang#8154 changelog: Don't lint `manual_swap` when a field access involves auto-deref
2 parents 3ea7784 + a7097b8 commit 0e28e38

File tree

4 files changed

+100
-1
lines changed

4 files changed

+100
-1
lines changed

clippy_utils/src/lib.rs

+21
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,19 @@ fn projection_stack<'a, 'hir>(mut e: &'a Expr<'hir>) -> (Vec<&'a Expr<'hir>>, &'
621621
(result, root)
622622
}
623623

624+
/// Gets the mutability of the custom deref adjustment, if any.
625+
pub fn expr_custom_deref_adjustment(cx: &LateContext<'_>, e: &Expr<'_>) -> Option<Mutability> {
626+
cx.typeck_results()
627+
.expr_adjustments(e)
628+
.iter()
629+
.find_map(|a| match a.kind {
630+
Adjust::Deref(Some(d)) => Some(Some(d.mutbl)),
631+
Adjust::Deref(None) => None,
632+
_ => Some(None),
633+
})
634+
.and_then(|x| x)
635+
}
636+
624637
/// Checks if two expressions can be mutably borrowed simultaneously
625638
/// and they aren't dependent on borrowing same thing twice
626639
pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -> bool {
@@ -629,7 +642,15 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -
629642
if !eq_expr_value(cx, r1, r2) {
630643
return true;
631644
}
645+
if expr_custom_deref_adjustment(cx, r1).is_some() || expr_custom_deref_adjustment(cx, r2).is_some() {
646+
return false;
647+
}
648+
632649
for (x1, x2) in s1.iter().zip(s2.iter()) {
650+
if expr_custom_deref_adjustment(cx, x1).is_some() || expr_custom_deref_adjustment(cx, x2).is_some() {
651+
return false;
652+
}
653+
633654
match (&x1.kind, &x2.kind) {
634655
(ExprKind::Field(_, i1), ExprKind::Field(_, i2)) => {
635656
if i1 != i2 {

tests/ui/swap.fixed

+33
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,36 @@ fn main() {
122122

123123
; std::mem::swap(&mut c.0, &mut a);
124124
}
125+
126+
fn issue_8154() {
127+
struct S1 {
128+
x: i32,
129+
y: i32,
130+
}
131+
struct S2(S1);
132+
struct S3<'a, 'b>(&'a mut &'b mut S1);
133+
134+
impl std::ops::Deref for S2 {
135+
type Target = S1;
136+
fn deref(&self) -> &Self::Target {
137+
&self.0
138+
}
139+
}
140+
impl std::ops::DerefMut for S2 {
141+
fn deref_mut(&mut self) -> &mut Self::Target {
142+
&mut self.0
143+
}
144+
}
145+
146+
// Don't lint. `s.0` is mutably borrowed by `s.x` and `s.y` via the deref impl.
147+
let mut s = S2(S1 { x: 0, y: 0 });
148+
let t = s.x;
149+
s.x = s.y;
150+
s.y = t;
151+
152+
// Accessing through a mutable reference is fine
153+
let mut s = S1 { x: 0, y: 0 };
154+
let mut s = &mut s;
155+
let s = S3(&mut s);
156+
std::mem::swap(&mut s.0.x, &mut s.0.y);
157+
}

tests/ui/swap.rs

+35
Original file line numberDiff line numberDiff line change
@@ -144,3 +144,38 @@ fn main() {
144144
c.0 = a;
145145
a = t;
146146
}
147+
148+
fn issue_8154() {
149+
struct S1 {
150+
x: i32,
151+
y: i32,
152+
}
153+
struct S2(S1);
154+
struct S3<'a, 'b>(&'a mut &'b mut S1);
155+
156+
impl std::ops::Deref for S2 {
157+
type Target = S1;
158+
fn deref(&self) -> &Self::Target {
159+
&self.0
160+
}
161+
}
162+
impl std::ops::DerefMut for S2 {
163+
fn deref_mut(&mut self) -> &mut Self::Target {
164+
&mut self.0
165+
}
166+
}
167+
168+
// Don't lint. `s.0` is mutably borrowed by `s.x` and `s.y` via the deref impl.
169+
let mut s = S2(S1 { x: 0, y: 0 });
170+
let t = s.x;
171+
s.x = s.y;
172+
s.y = t;
173+
174+
// Accessing through a mutable reference is fine
175+
let mut s = S1 { x: 0, y: 0 };
176+
let mut s = &mut s;
177+
let s = S3(&mut s);
178+
let t = s.0.x;
179+
s.0.x = s.0.y;
180+
s.0.y = t;
181+
}

tests/ui/swap.stderr

+11-1
Original file line numberDiff line numberDiff line change
@@ -108,5 +108,15 @@ LL | | a = c.0;
108108
|
109109
= note: or maybe you should use `std::mem::replace`?
110110

111-
error: aborting due to 12 previous errors
111+
error: this looks like you are swapping `s.0.x` and `s.0.y` manually
112+
--> $DIR/swap.rs:178:5
113+
|
114+
LL | / let t = s.0.x;
115+
LL | | s.0.x = s.0.y;
116+
LL | | s.0.y = t;
117+
| |_____________^ help: try: `std::mem::swap(&mut s.0.x, &mut s.0.y)`
118+
|
119+
= note: or maybe you should use `std::mem::replace`?
120+
121+
error: aborting due to 13 previous errors
112122

0 commit comments

Comments
 (0)