Warn about explicit self-assignment #5894
Conversation
|
r? @Manishearth (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
Not locking this lint from merging, but I find this lint really helpful: Why not lint it in rustc ? |
|
I would expect that rustc version would have to be very conservative with Advice about this aspect would be appreciated. |
rustc is conservative to accept lints. You can always file an issue on rustc to uplift a lint, they're often open to that, but it's up to them |
clippy_lints/src/self_assignment.rs
Outdated
| } | ||
|
|
||
| /// Returns true if two expressions are the same and don't have any "obvious" side-effects. | ||
| fn is_same_expr<'tcx>(mut a: &Expr<'tcx>, mut b: &Expr<'tcx>, place_expr: bool) -> bool { |
There was a problem hiding this comment.
this should go in utils (and perhaps check if something like this doesn't already exist)
There was a problem hiding this comment.
The implementation is similar to SpanlessEq::eq_expr. The main difference is
that the one here returns false for expressions with side-effects (except for
those that might involve Deref, Index, IndexMut), while SpanlessEq has option
to exclude side-effects but only in the form of function calls.
For now I retained custom impl, but moved it to utils.
There was a problem hiding this comment.
Would it make sense to enhance SpanlessEq with an option to exclude all side effects?
There was a problem hiding this comment.
OK, after reviewing all uses of SpanlessEq::ignore_fn, it is clear that intent
behind it is the same, even if details differed previously. I extended it to
consider additional side effects, renamed ignore_fn to deny_side_effects,
and introduced eq_expr_value as a short-cut.
|
☔ The latest upstream changes (presumably #5903) made this pull request unmergeable. Please resolve the merge conflicts. |
Introduce `eq_expr_value(cx, a, b)` as a shortcut for `SpanlessEq::new(cx).ignore_fn().eq_expr(cx, a, b)`. No functional changes intended.
No functional changes intended.
Warn about assignments where left-hand side place expression is the same
as right-hand side value expression. For example, warn about assignment in:
```rust
pub struct Event {
id: usize,
x: i32,
y: i32,
}
pub fn copy_position(a: &mut Event, b: &Event) {
a.x = b.x;
a.y = a.y;
}
```
|
@bors r+ sweet, this is much cleaner, thanks |
|
📌 Commit 4f4abf4 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Warn about assignments where left-hand side place expression is the same
as right-hand side value expression. For example, warn about assignment in:
changelog: New lint
self_assignment, checks for explicit self-assignments.