Apply missing transmute annotations lint - #161677
Conversation
The clippy suggestion was actually wrong, as the lint does not handle pattern_types
|
cc @Amanieu, @folkertdev, @sayantn Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred in library/core/src/fmt/rt.rs cc @m-ou-se |
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| unsafe { crate::mem::transmute::<_, u128>(a) == crate::mem::transmute::<_, u128>(b) } | ||
| unsafe { | ||
| crate::mem::transmute::<crate::any::TypeId, u128>(a) | ||
| == crate::mem::transmute::<crate::any::TypeId, u128>(b) |
There was a problem hiding this comment.
FWIW I am not sure this makes the code better. The type of a and b is right there in the function signature, what is the point in repeating it?
There was a problem hiding this comment.
maybe
a) we could add an expect/allow instead.
b) the lint should not fire in this case. Right now, the lint does not fire if the transmute is the only element in the function, which is not the case here, it is a bit more complex.
Best reviewed case by case. Special care should be taken when involving pointers.
The missing_transmute_annotations lint.
The point of the lint is that in order to ensure that the safety invariant holds, the transmute source and destination should be explicitly fixed.
Remarks:
I kept the initial suggestion and the fixed suggestion in separate commit so that the issue can be observed easily and the clippy lint can be fixed before even considering enabling it.
Notes:
define_valid_range, the transmute suggestion was wrong because:pattern_types!are strangely formatted. The issue is being worked on inmissing_transmute_annotations: handle pattern_types in suggestion rust-clippy#17613.Notably, the proper fix may be to change the display impl for pattern_types (see
missing_transmute_annotations: handle pattern_types in suggestion rust-clippy#17613 (comment) by @ada4aYet I still propose a fix because this case seems clear cut, it may be an exception to the iffiness rule.
The suggestion in main...GTimothy:rust:missing-transmute-annotations#diff-4fbe4e316a3fa623f3edea42a8d3ab2ec54de32c3ec8e46e5d5d58a562a86813 was muted because it was very very complex and inside a macro.
core_archsuggestions are all of the same nature, and the suggestions are wrong because they:I have no clue how this dark magic works:
Related: "Enforce even more library clippy lints in CI" by @ChrisDenton: #161328