rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.28k stars 1.52k forks source link

review all `expr_ty` calls #1751

Open oli-obk opened 7 years ago

oli-obk commented 7 years ago

Often they should be expr_ty_adjusted.

eddyb commented 7 years ago

I wanted to open the same issue, glad other people have noticed!

The advice I have is that if you have an operand, i.e. a sub-expression of a call, an unary or binary operator, any kind of ADT constructor, and the type wanted is that "passed" to the operation, then you definitely want expr_ty_adjusted, which is correct even if rustc needs to insert coercions. This also includes let initializers and the tail of a block, as those can have coercions as well.

For lvalue expressions (e.g. a field access), it's trickier, because you might want the "final" type just like for operands (e.g. the struct type the field came from), or to peek at the (autoderef) process.

Actually, if you don't care about which coercions were used, you most likely want expr_ty_adjusted. The main exception I can think of is how x == y may autoref (and be PartialEq::eq(&x, &y) - this also applies to any comparison operator, not just equality), but that's only a problem if you want to extract the LHS and RHS types that go into the trait (e.g. <T as PartialEq<U>>), but you have Substs for that, which may be more convenient to use - not to mention that the RHS of a binary operator can actually have coercions applied to it, so you really have to look at the Substs.