Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint using blocks in compound expression operators #4756

Open
Aaron1011 opened this issue Oct 30, 2019 · 1 comment
Open

Lint using blocks in compound expression operators #4756

Aaron1011 opened this issue Oct 30, 2019 · 1 comment
Labels
A-lint Area: New lints L-correctness Lint: Belongs in the correctness lint group

Comments

@Aaron1011
Copy link
Member

Aaron1011 commented Oct 30, 2019

Consider the following code:

#[derive(Copy, Clone)]
struct MyAdd;

impl std::ops::AddAssign<MyAdd> for MyAdd {
    fn add_assign(&mut self, other: MyAdd) {}
}

fn main() {
    let lhs = &mut MyAdd;
    let rhs = MyAdd;
    *{println!("LHS"); lhs} += {println!("RHS"); rhs};
}

This code uses a block express as both the LHS and RHS of a compound assignment operator (+=). This code is not only difficult to read - its execution order depends on the type of the expression produced by the block (see rust-lang/rust#28160 and rust-lang/rust#61572).

As written, this code prints:

LHS
RHS

However, this nearly identical code:

fn main() {
    let lhs = &mut 25;
    let rhs = 30;
    *{println!("LHS"); lhs} += {println!("RHS"); rhs};
}

prints:

RHS
LHS

That is, the evaluation order of the LHS and RHS is dependent on whether or not the type used is a primitive or not.

Using a block expression with a compound assignment operator is a bad idea in and of itself - it's very difficult to read, and makes it unclear what is actually happening. However, the inconsistent evaluation order makes this even worse - a seemly unrelated change (e.g. changing the type of a variable earlier up in a function) can result in the execution order of other expressions changing.

It would be useful if Clippy were to lint the use of anything other than a variable, literal, or (chain of) method calls with a compound assignment operator.

@flip1995 flip1995 added L-correctness Lint: Belongs in the correctness lint group A-lint Area: New lints labels Oct 31, 2019
@camsteffen
Copy link
Contributor

I think the block on the LHS is more egregious than the one on the RHS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints L-correctness Lint: Belongs in the correctness lint group
Projects
None yet
Development

No branches or pull requests

3 participants