fix f16/f128 lints #14956
fix f16/f128 lints #14956
Conversation
rustbot
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
| fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 { | ||
| match typ.kind() { | ||
| ty::Float(FloatTy::F32) => 23, | ||
| ty::Float(FloatTy::F64) | ty::Infer(InferTy::FloatVar(_)) => 52, |
There was a problem hiding this comment.
ty::Infer(InferTy::FloatVar(_)) => 52 should be unnecessary, is that right?
| pub(super) fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_>) -> u64 { | ||
| match typ.kind() { | ||
| ty::Int(i) => match i { | ||
| IntTy::Isize => tcx.data_layout.pointer_size.bits(), | ||
| IntTy::I8 => 8, | ||
| IntTy::I16 => 16, | ||
| IntTy::I32 => 32, | ||
| IntTy::I64 => 64, | ||
| IntTy::I128 => 128, | ||
| }, | ||
| ty::Uint(i) => match i { | ||
| UintTy::Usize => tcx.data_layout.pointer_size.bits(), | ||
| UintTy::U8 => 8, | ||
| UintTy::U16 => 16, | ||
| UintTy::U32 => 32, | ||
| UintTy::U64 => 64, | ||
| UintTy::U128 => 128, | ||
| }, | ||
| _ => unreachable!(), | ||
| } | ||
| } |
There was a problem hiding this comment.
You are reverting (here and at other places) changes that were committed yesterday. Please don't do that. And please explain what your PR is doing, what it fixes, why it is necessary, and when you change existing things why your solution is better than the one in place. For example, why would panicking (through unreachable!()) be best than returning None here? The Option version lets us check if this is an integral type at the same time, which is almost always necessary from the caller point of view anyway. Also, it is shorter by using i.bit_width() rather than hard-coding every case.
There was a problem hiding this comment.
According to your approach, how can we make use of int_ty_to_nbits, float_ty_to_nbits, and float_ty_to_mantissa_nbits at the same time? Should all three return Option, and then handle the None cases externally?
There was a problem hiding this comment.
I don't know what you're trying to achieve, so this is a difficult question to answer.
4 participants
changelog: none