Skip to content

Commit b6b97a7

Browse files
llogiqllogiq
authored
Fix assign_op_pattern FP on unstable const trait (#14886)
Closes #14871 ---- changelog: [`assign_op_pattern`]: fix false positive on unstable const trait
2 parents 24a2a66 + 03af373 commit b6b97a7

File tree

6 files changed

+90
-15
lines changed

6 files changed

+90
-15
lines changed

‎clippy_lints/src/operators/assign_op_pattern.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::msrvs::Msrv;
3+
use clippy_utils::qualify_min_const_fn::is_stable_const_fn;
24
use clippy_utils::source::SpanRangeExt;
35
use clippy_utils::ty::implements_trait;
46
use clippy_utils::visitors::for_each_expr_without_closures;
5-
use clippy_utils::{binop_traits, eq_expr_value, trait_ref_of_method};
7+
use clippy_utils::{binop_traits, eq_expr_value, is_in_const_context, trait_ref_of_method};
68
use core::ops::ControlFlow;
79
use rustc_errors::Applicability;
810
use rustc_hir as hir;
@@ -19,6 +21,7 @@ pub(super) fn check<'tcx>(
1921
expr: &'tcx hir::Expr<'_>,
2022
assignee: &'tcx hir::Expr<'_>,
2123
e: &'tcx hir::Expr<'_>,
24+
msrv: Msrv,
2225
) {
2326
if let hir::ExprKind::Binary(op, l, r) = &e.kind {
2427
let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| {
@@ -40,6 +43,15 @@ pub(super) fn check<'tcx>(
4043
return;
4144
}
4245
}
46+
47+
// Skip if the trait is not stable in const contexts
48+
if is_in_const_context(cx)
49+
&& let Some(binop_id) = cx.tcx.associated_item_def_ids(trait_id).first()
50+
&& !is_stable_const_fn(cx, *binop_id, msrv)
51+
{
52+
return;
53+
}
54+
4355
span_lint_and_then(
4456
cx,
4557
ASSIGN_OP_PATTERN,

‎clippy_lints/src/operators/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
919919
modulo_arithmetic::check(cx, e, bin_op, lhs, rhs, false);
920920
},
921921
ExprKind::Assign(lhs, rhs, _) => {
922-
assign_op_pattern::check(cx, e, lhs, rhs);
922+
assign_op_pattern::check(cx, e, lhs, rhs, self.msrv);
923923
self_assignment::check(cx, e, lhs, rhs);
924924
},
925925
ExprKind::Unary(op, arg) => {

‎clippy_utils/src/qualify_min_const_fn.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,8 @@ fn check_terminator<'tcx>(
393393
}
394394
}
395395

396-
fn is_stable_const_fn(cx: &LateContext<'_>, def_id: DefId, msrv: Msrv) -> bool {
396+
/// Checks if the given `def_id` is a stable const fn, in respect to the given MSRV.
397+
pub fn is_stable_const_fn(cx: &LateContext<'_>, def_id: DefId, msrv: Msrv) -> bool {
397398
cx.tcx.is_const_fn(def_id)
398399
&& cx
399400
.tcx

‎tests/ui/assign_ops.fixed

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![allow(clippy::useless_vec)]
22
#![warn(clippy::assign_op_pattern)]
3+
#![feature(const_trait_impl, const_ops)]
34

45
use core::num::Wrapping;
56
use std::ops::{Mul, MulAssign};
@@ -73,3 +74,33 @@ impl MulAssign<i64> for Wrap {
7374
*self = *self * rhs
7475
}
7576
}
77+
78+
mod issue14871 {
79+
80+
use std::ops::{Add, AddAssign};
81+
82+
pub trait Number: Copy + Add<Self, Output = Self> + AddAssign {
83+
const ZERO: Self;
84+
const ONE: Self;
85+
}
86+
87+
#[const_trait]
88+
pub trait NumberConstants {
89+
fn constant(value: usize) -> Self;
90+
}
91+
92+
impl<T> const NumberConstants for T
93+
where
94+
T: Number + ~const core::ops::Add,
95+
{
96+
fn constant(value: usize) -> Self {
97+
let mut res = Self::ZERO;
98+
let mut count = 0;
99+
while count < value {
100+
res = res + Self::ONE;
101+
count += 1;
102+
}
103+
res
104+
}
105+
}
106+
}

‎tests/ui/assign_ops.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![allow(clippy::useless_vec)]
22
#![warn(clippy::assign_op_pattern)]
3+
#![feature(const_trait_impl, const_ops)]
34

45
use core::num::Wrapping;
56
use std::ops::{Mul, MulAssign};
@@ -73,3 +74,33 @@ impl MulAssign<i64> for Wrap {
7374
*self = *self * rhs
7475
}
7576
}
77+
78+
mod issue14871 {
79+
80+
use std::ops::{Add, AddAssign};
81+
82+
pub trait Number: Copy + Add<Self, Output = Self> + AddAssign {
83+
const ZERO: Self;
84+
const ONE: Self;
85+
}
86+
87+
#[const_trait]
88+
pub trait NumberConstants {
89+
fn constant(value: usize) -> Self;
90+
}
91+
92+
impl<T> const NumberConstants for T
93+
where
94+
T: Number + ~const core::ops::Add,
95+
{
96+
fn constant(value: usize) -> Self {
97+
let mut res = Self::ZERO;
98+
let mut count = 0;
99+
while count < value {
100+
res = res + Self::ONE;
101+
count += 1;
102+
}
103+
res
104+
}
105+
}
106+
}

‎tests/ui/assign_ops.stderr

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: manual implementation of an assign operation
2-
--> tests/ui/assign_ops.rs:9:5
2+
--> tests/ui/assign_ops.rs:10:5
33
|
44
LL | a = a + 1;
55
| ^^^^^^^^^ help: replace it with: `a += 1`
@@ -8,67 +8,67 @@ LL | a = a + 1;
88
= help: to override `-D warnings` add `#[allow(clippy::assign_op_pattern)]`
99

1010
error: manual implementation of an assign operation
11-
--> tests/ui/assign_ops.rs:11:5
11+
--> tests/ui/assign_ops.rs:12:5
1212
|
1313
LL | a = 1 + a;
1414
| ^^^^^^^^^ help: replace it with: `a += 1`
1515

1616
error: manual implementation of an assign operation
17-
--> tests/ui/assign_ops.rs:13:5
17+
--> tests/ui/assign_ops.rs:14:5
1818
|
1919
LL | a = a - 1;
2020
| ^^^^^^^^^ help: replace it with: `a -= 1`
2121

2222
error: manual implementation of an assign operation
23-
--> tests/ui/assign_ops.rs:15:5
23+
--> tests/ui/assign_ops.rs:16:5
2424
|
2525
LL | a = a * 99;
2626
| ^^^^^^^^^^ help: replace it with: `a *= 99`
2727

2828
error: manual implementation of an assign operation
29-
--> tests/ui/assign_ops.rs:17:5
29+
--> tests/ui/assign_ops.rs:18:5
3030
|
3131
LL | a = 42 * a;
3232
| ^^^^^^^^^^ help: replace it with: `a *= 42`
3333

3434
error: manual implementation of an assign operation
35-
--> tests/ui/assign_ops.rs:19:5
35+
--> tests/ui/assign_ops.rs:20:5
3636
|
3737
LL | a = a / 2;
3838
| ^^^^^^^^^ help: replace it with: `a /= 2`
3939

4040
error: manual implementation of an assign operation
41-
--> tests/ui/assign_ops.rs:21:5
41+
--> tests/ui/assign_ops.rs:22:5
4242
|
4343
LL | a = a % 5;
4444
| ^^^^^^^^^ help: replace it with: `a %= 5`
4545

4646
error: manual implementation of an assign operation
47-
--> tests/ui/assign_ops.rs:23:5
47+
--> tests/ui/assign_ops.rs:24:5
4848
|
4949
LL | a = a & 1;
5050
| ^^^^^^^^^ help: replace it with: `a &= 1`
5151

5252
error: manual implementation of an assign operation
53-
--> tests/ui/assign_ops.rs:30:5
53+
--> tests/ui/assign_ops.rs:31:5
5454
|
5555
LL | s = s + "bla";
5656
| ^^^^^^^^^^^^^ help: replace it with: `s += "bla"`
5757

5858
error: manual implementation of an assign operation
59-
--> tests/ui/assign_ops.rs:35:5
59+
--> tests/ui/assign_ops.rs:36:5
6060
|
6161
LL | a = a + Wrapping(1u32);
6262
| ^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a += Wrapping(1u32)`
6363

6464
error: manual implementation of an assign operation
65-
--> tests/ui/assign_ops.rs:38:5
65+
--> tests/ui/assign_ops.rs:39:5
6666
|
6767
LL | v[0] = v[0] + v[1];
6868
| ^^^^^^^^^^^^^^^^^^ help: replace it with: `v[0] += v[1]`
6969

7070
error: manual implementation of an assign operation
71-
--> tests/ui/assign_ops.rs:51:5
71+
--> tests/ui/assign_ops.rs:52:5
7272
|
7373
LL | buf = buf + cows.clone();
7474
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `buf += cows.clone()`

0 commit comments

Comments
 (0)