Open CAD97 opened 4 years ago
Other than special casing struct initializers, the other potential change is to not fire eval_order_dependence
when the write is to a non-mut
let
binding, as the place can only ever be assigned once and must be written before the read, so only one interpretation of the ordering is sound.
It hit me today. Here's a simplified version:
async fn one() -> u32 { 1 }
async fn two() -> u32 { 2 }
pub struct TwoInts {
a: u32,
b: u32,
}
pub async fn ff() -> TwoInts {
TwoInts {
a: one().await,
b: two().await,
}
}
gives
warning: unsequenced read of a variable
--> src/lib.rs:12:12
|
12 | b: two().await,
| ^^^^^^^^^^^
|
= note: `#[warn(clippy::eval_order_dependence)]` on by default
note: whether read occurs before this write depends on evaluation order
--> src/lib.rs:11:12
|
11 | a: one().await,
| ^^^^^^^^^^^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence
warning: unsequenced read of a variable
--> src/lib.rs:11:12
|
11 | a: one().await,
| ^^^^^^^^^^^
|
note: whether read occurs before this write depends on evaluation order
--> src/lib.rs:12:12
|
12 | b: two().await,
| ^^^^^^^^^^^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence
IMHO this is safe and should not be linted.
@flip1995 Could you add the correct labels as well as maybe give an instruction on how to fix this? I would work on this, but currently have no plan to start where :)
@hellow554 I added the labels. For the instruction on how to fix this, it is a bit more complicated. This is probably a really easy fix, the question is, where it has to be done. You probably have to just add if some_span.from_expansion() { return }
somewhere in the linting code. If you are unable to find the place where this should be added, just ping me again and I'll take a look at the code.
I tried a few things yesterday, but they either broke existing tests (not sure if legitmate) oder hadn't the desirede effect. I don't think that this should be too hard 😅 But yeah, a look would be great @flip1995
Uh, I think this is harder than I thought. For once, the lint is triggered inside a visitor, which is a bad idea most of the time. This is also the reason why just adding a from_expansion
won't work. Fixing this may even mean rewriting some of the linting code... (At least I think so)
The lint has been renamed to mixed_read_write_expression
as of #8621.
It's become idiomatic for
syn
-based parsers to parse members of each AST struct directly into the struct initializer, including e.g.bracketed!
which initializes a place outside of the initializer. Thebracketed!
example:clippy gives a
eval_order_dependence
warning here:I believe that struct initializer syntax is in fact strongly specified to evaluate the member expressions in source order. And in this case, the order of the write and read can only be this, as the place assigned is both non-
mut
and not yet initialized before the write.(Note that
ParseStream
is&'_ ParseBuffer<'_>
which does include internal mutability but does so through&Cell
, so the false negative on internal mutability ordering here is expected and likely unpreventable.)