From 8cde354f0bb0d686882b32883c36c252ee367967 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Tue, 18 Jun 2024 11:46:26 -0500 Subject: [PATCH] Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s This removes the ICE codepaths for `f16` and `f128` in Clippy. `rustc_apfloat` is used as a dependency for the parsing of these types, since their `FromStr` implementation will not be available in the standard library for a while. --- clippy_lints/src/casts/cast_nan_to_int.rs | 1 + clippy_lints/src/float_literal.rs | 7 ++- clippy_lints/src/lib.rs | 2 + clippy_lints/src/manual_float_methods.rs | 2 + clippy_lints/src/operators/float_cmp.rs | 1 + .../src/operators/modulo_arithmetic.rs | 1 + clippy_lints/src/zero_div_zero.rs | 1 + clippy_utils/Cargo.toml | 2 + clippy_utils/src/consts.rs | 43 ++++++++++++++++--- clippy_utils/src/lib.rs | 2 + 10 files changed, 52 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/casts/cast_nan_to_int.rs b/clippy_lints/src/casts/cast_nan_to_int.rs index 1743ce71add..5bc8692c289 100644 --- a/clippy_lints/src/casts/cast_nan_to_int.rs +++ b/clippy_lints/src/casts/cast_nan_to_int.rs @@ -21,6 +21,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, fn is_known_nan(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { match constant(cx, cx.typeck_results(), e) { + // FIXME(f16_f128): add these types when nan checks are available on all platforms Some(Constant::F64(n)) => n.is_nan(), Some(Constant::F32(n)) => n.is_nan(), _ => false, diff --git a/clippy_lints/src/float_literal.rs b/clippy_lints/src/float_literal.rs index 4d301daabe4..2261fcdbdab 100644 --- a/clippy_lints/src/float_literal.rs +++ b/clippy_lints/src/float_literal.rs @@ -141,18 +141,17 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { #[must_use] fn max_digits(fty: FloatTy) -> u32 { match fty { - // FIXME(f16_f128): replace the magic numbers once `{f16,f128}::DIGITS` are available - FloatTy::F16 => 3, + FloatTy::F16 => f16::DIGITS, FloatTy::F32 => f32::DIGITS, FloatTy::F64 => f64::DIGITS, - FloatTy::F128 => 33, + FloatTy::F128 => f128::DIGITS, } } /// Counts the digits excluding leading zeros #[must_use] fn count_digits(s: &str) -> usize { - // Note that s does not contain the f32/64 suffix, and underscores have been stripped + // Note that s does not contain the `f{16,32,64,128}` suffix, and underscores have been stripped s.chars() .filter(|c| *c != '-' && *c != '.') .take_while(|c| *c != 'e' && *c != 'E') diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c65581d5203..ef322786dbc 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1,6 +1,8 @@ #![feature(array_windows)] #![feature(binary_heap_into_iter_sorted)] #![feature(box_patterns)] +#![feature(f128)] +#![feature(f16)] #![feature(if_let_guard)] #![feature(iter_intersperse)] #![feature(let_chains)] diff --git a/clippy_lints/src/manual_float_methods.rs b/clippy_lints/src/manual_float_methods.rs index 72cf1d7a354..89eea0b4456 100644 --- a/clippy_lints/src/manual_float_methods.rs +++ b/clippy_lints/src/manual_float_methods.rs @@ -156,6 +156,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { fn is_infinity(constant: &Constant<'_>) -> bool { match constant { + // FIXME(f16_f128): add f16 and f128 when constants are available Constant::F32(float) => *float == f32::INFINITY, Constant::F64(float) => *float == f64::INFINITY, _ => false, @@ -164,6 +165,7 @@ fn is_infinity(constant: &Constant<'_>) -> bool { fn is_neg_infinity(constant: &Constant<'_>) -> bool { match constant { + // FIXME(f16_f128): add f16 and f128 when constants are available Constant::F32(float) => *float == f32::NEG_INFINITY, Constant::F64(float) => *float == f64::NEG_INFINITY, _ => false, diff --git a/clippy_lints/src/operators/float_cmp.rs b/clippy_lints/src/operators/float_cmp.rs index 0561739d160..faab79de9d3 100644 --- a/clippy_lints/src/operators/float_cmp.rs +++ b/clippy_lints/src/operators/float_cmp.rs @@ -86,6 +86,7 @@ fn get_lint_and_message(is_local: bool, is_comparing_arrays: bool) -> (&'static fn is_allowed(val: &Constant<'_>) -> bool { match val { + // FIXME(f16_f128): add when equality check is available on all platforms &Constant::F32(f) => f == 0.0 || f.is_infinite(), &Constant::F64(f) => f == 0.0 || f.is_infinite(), Constant::Vec(vec) => vec.iter().all(|f| match f { diff --git a/clippy_lints/src/operators/modulo_arithmetic.rs b/clippy_lints/src/operators/modulo_arithmetic.rs index c56518ac72a..d65fffac5a8 100644 --- a/clippy_lints/src/operators/modulo_arithmetic.rs +++ b/clippy_lints/src/operators/modulo_arithmetic.rs @@ -79,6 +79,7 @@ fn analyze_operand(operand: &Expr<'_>, cx: &LateContext<'_>, expr: &Expr<'_>) -> }, _ => {}, }, + // FIXME(f16_f128): add when casting is available on all platforms Some(Constant::F32(f)) => { return Some(floating_point_operand_info(&f)); }, diff --git a/clippy_lints/src/zero_div_zero.rs b/clippy_lints/src/zero_div_zero.rs index 662242f6196..60d8a13d359 100644 --- a/clippy_lints/src/zero_div_zero.rs +++ b/clippy_lints/src/zero_div_zero.rs @@ -38,6 +38,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // do something like 0.0/(2.0 - 2.0), but it would be nice to warn on that case too. && let Some(lhs_value) = constant_simple(cx, cx.typeck_results(), left) && let Some(rhs_value) = constant_simple(cx, cx.typeck_results(), right) + // FIXME(f16_f128): add these types when eq is available on all platforms && (Constant::F32(0.0) == lhs_value || Constant::F64(0.0) == lhs_value) && (Constant::F32(0.0) == rhs_value || Constant::F64(0.0) == rhs_value) { diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index 3a3aeb88216..6e53ff3ee6e 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -9,6 +9,8 @@ clippy_config = { path = "../clippy_config" } arrayvec = { version = "0.7", default-features = false } itertools = "0.12" rustc-semver = "1.1" +# FIXME(f16_f128): remove when no longer needed for parsing +rustc_apfloat = "0.2.0" [features] deny-warnings = ["clippy_config/deny-warnings"] diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index cfd142fe1ff..681c86f76d0 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -3,6 +3,8 @@ use crate::source::{get_source_text, walk_span_to_context}; use crate::{clip, is_direct_expn_of, sext, unsext}; +use rustc_apfloat::ieee::{Half, Quad}; +use rustc_apfloat::Float; use rustc_ast::ast::{self, LitFloatType, LitKind}; use rustc_data_structures::sync::Lrc; use rustc_hir::def::{DefKind, Res}; @@ -33,10 +35,14 @@ pub enum Constant<'tcx> { Char(char), /// An integer's bit representation. Int(u128), + /// An `f16`. + F16(f16), /// An `f32`. F32(f32), /// An `f64`. F64(f64), + /// An `f128`. + F128(f128), /// `true` or `false`. Bool(bool), /// An array of constants. @@ -161,12 +167,19 @@ fn hash(&self, state: &mut H) Self::Int(i) => { i.hash(state); }, + Self::F16(f) => { + // FIXME(f16_f128): once conversions to/from `f128` are available on all platforms, + f.to_bits().hash(state); + }, Self::F32(f) => { f64::from(f).to_bits().hash(state); }, Self::F64(f) => { f.to_bits().hash(state); }, + Self::F128(f) => { + f.to_bits().hash(state); + }, Self::Bool(b) => { b.hash(state); }, @@ -268,6 +281,16 @@ pub fn peel_refs(mut self) -> Self { } self } + + fn parse_f16(s: &str) -> Self { + let f: Half = s.parse().unwrap(); + Self::F16(f16::from_bits(f.to_bits().try_into().unwrap())) + } + + fn parse_f128(s: &str) -> Self { + let f: Quad = s.parse().unwrap(); + Self::F128(f128::from_bits(f.to_bits())) + } } /// Parses a `LitKind` to a `Constant`. @@ -279,16 +302,17 @@ pub fn lit_to_mir_constant<'tcx>(lit: &LitKind, ty: Option>) -> Constan LitKind::Char(c) => Constant::Char(c), LitKind::Int(n, _) => Constant::Int(n.get()), LitKind::Float(ref is, LitFloatType::Suffixed(fty)) => match fty { - ast::FloatTy::F16 => unimplemented!("f16_f128"), + // FIXME(f16_f128): just use `parse()` directly when available for `f16`/`f128` + ast::FloatTy::F16 => Constant::parse_f16(is.as_str()), ast::FloatTy::F32 => Constant::F32(is.as_str().parse().unwrap()), ast::FloatTy::F64 => Constant::F64(is.as_str().parse().unwrap()), - ast::FloatTy::F128 => unimplemented!("f16_f128"), + ast::FloatTy::F128 => Constant::parse_f128(is.as_str()), }, LitKind::Float(ref is, LitFloatType::Unsuffixed) => match ty.expect("type of float is known").kind() { - ty::Float(FloatTy::F16) => unimplemented!("f16_f128"), + ty::Float(FloatTy::F16) => Constant::parse_f16(is.as_str()), ty::Float(FloatTy::F32) => Constant::F32(is.as_str().parse().unwrap()), ty::Float(FloatTy::F64) => Constant::F64(is.as_str().parse().unwrap()), - ty::Float(FloatTy::F128) => unimplemented!("f16_f128"), + ty::Float(FloatTy::F128) => Constant::parse_f128(is.as_str()), _ => bug!(), }, LitKind::Bool(b) => Constant::Bool(b), @@ -625,15 +649,19 @@ fn index(&mut self, lhs: &'_ Expr<'_>, index: &'_ Expr<'_>) -> Option match vec.get(index as usize) { + Some(Constant::F16(x)) => Some(Constant::F16(*x)), Some(Constant::F32(x)) => Some(Constant::F32(*x)), Some(Constant::F64(x)) => Some(Constant::F64(*x)), + Some(Constant::F128(x)) => Some(Constant::F128(*x)), _ => None, }, (Some(Constant::Vec(vec)), _) => { if !vec.is_empty() && vec.iter().all(|x| *x == vec[0]) { match vec.first() { + Some(Constant::F16(x)) => Some(Constant::F16(*x)), Some(Constant::F32(x)) => Some(Constant::F32(*x)), Some(Constant::F64(x)) => Some(Constant::F64(*x)), + Some(Constant::F128(x)) => Some(Constant::F128(*x)), _ => None, } } else { @@ -760,6 +788,7 @@ fn binop(&mut self, op: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> Option None, }, + // FIXME(f16_f128): add these types when binary operations are available on all platforms (Constant::F32(l), Some(Constant::F32(r))) => match op.node { BinOpKind::Add => Some(Constant::F32(l + r)), BinOpKind::Sub => Some(Constant::F32(l - r)), @@ -813,8 +842,10 @@ pub fn mir_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> ty::Adt(adt_def, _) if adt_def.is_struct() => Some(Constant::Adt(result)), ty::Bool => Some(Constant::Bool(int == ScalarInt::TRUE)), ty::Uint(_) | ty::Int(_) => Some(Constant::Int(int.to_bits(int.size()))), + ty::Float(FloatTy::F16) => Some(Constant::F16(f16::from_bits(int.into()))), ty::Float(FloatTy::F32) => Some(Constant::F32(f32::from_bits(int.into()))), ty::Float(FloatTy::F64) => Some(Constant::F64(f64::from_bits(int.into()))), + ty::Float(FloatTy::F128) => Some(Constant::F128(f128::from_bits(int.into()))), ty::RawPtr(_, _) => Some(Constant::RawPtr(int.to_bits(int.size()))), _ => None, }, @@ -835,10 +866,10 @@ pub fn mir_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> let range = alloc_range(offset + size * idx, size); let val = alloc.read_scalar(&lcx.tcx, range, /* read_provenance */ false).ok()?; res.push(match flt { - FloatTy::F16 => unimplemented!("f16_f128"), + FloatTy::F16 => Constant::F16(f16::from_bits(val.to_u16().ok()?)), FloatTy::F32 => Constant::F32(f32::from_bits(val.to_u32().ok()?)), FloatTy::F64 => Constant::F64(f64::from_bits(val.to_u64().ok()?)), - FloatTy::F128 => unimplemented!("f16_f128"), + FloatTy::F128 => Constant::F128(f128::from_bits(val.to_u128().ok()?)), }); } Some(Constant::Vec(res)) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 7dc341ec8d7..6848e8e5c30 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1,6 +1,8 @@ #![feature(array_chunks)] #![feature(box_patterns)] #![feature(control_flow_enum)] +#![feature(f128)] +#![feature(f16)] #![feature(if_let_guard)] #![feature(let_chains)] #![feature(lint_reasons)]