Skip to content

Commit

Permalink
Rollup merge of #126636 - tgross35:clippy-f16-f128-fixme, r=flip1995
Browse files Browse the repository at this point in the history
Resolve Clippy `f16` and `f128` `unimplemented!`/`FIXME`s

This was originally a PR against the Clippy repo, rust-lang/rust-clippy#12950

r? ``@flip1995``

Tracking issue: #116909
Fixes: #126636
  • Loading branch information
matthiaskrgr authored Jun 20, 2024
2 parents 586154b + 477e9e8 commit d3f5e7b
Show file tree
Hide file tree
Showing 55 changed files with 845 additions and 532 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@ dependencies = [
"clippy_config",
"itertools 0.12.1",
"rustc-semver",
"rustc_apfloat",
]

[[package]]
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_lints/src/casts/cast_nan_to_int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 3 additions & 4 deletions src/tools/clippy/clippy_lints/src/float_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,18 +141,17 @@ impl<'tcx> LateLintPass<'tcx> for FloatLiteral {
#[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')
Expand Down
2 changes: 2 additions & 0 deletions src/tools/clippy/clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
2 changes: 2 additions & 0 deletions src/tools/clippy/clippy_lints/src/manual_float_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods {

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,
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_lints/src/operators/float_cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
},
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_lints/src/zero_div_zero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ impl<'tcx> LateLintPass<'tcx> for ZeroDiv {
// 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)
{
Expand Down
2 changes: 2 additions & 0 deletions src/tools/clippy/clippy_utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
43 changes: 37 additions & 6 deletions src/tools/clippy/clippy_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -161,12 +167,19 @@ impl<'tcx> Hash for Constant<'tcx> {
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);
},
Expand Down Expand Up @@ -268,6 +281,16 @@ impl<'tcx> Constant<'tcx> {
}
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`.
Expand All @@ -279,16 +302,17 @@ pub fn lit_to_mir_constant<'tcx>(lit: &LitKind, ty: Option<Ty<'tcx>>) -> 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),
Expand Down Expand Up @@ -625,15 +649,19 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {

match (lhs, index) {
(Some(Constant::Vec(vec)), Some(Constant::Int(index))) => 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 {
Expand Down Expand Up @@ -760,6 +788,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
},
_ => 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)),
Expand Down Expand Up @@ -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,
},
Expand All @@ -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))
Expand Down
2 changes: 2 additions & 0 deletions src/tools/clippy/clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ fn main() {

a.sort_unstable();

// FIXME(f16_f128): add a clamp test once the function is available
let _ = 2.0f32.clamp(3.0f32, 4.0f32);
let _ = 2.0f64.clamp(3.0f64, 4.0f64);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,61 +28,61 @@ LL | a.sort_unstable();
| ^^^^^^^^^^^^^^^^^

error: use of a disallowed method `f32::clamp`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:43:13
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:44:13
|
LL | let _ = 2.0f32.clamp(3.0f32, 4.0f32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: use of a disallowed method `regex::Regex::new`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:46:61
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:47:61
|
LL | let indirect: fn(&str) -> Result<Regex, regex::Error> = Regex::new;
| ^^^^^^^^^^

error: use of a disallowed method `f32::clamp`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:49:28
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:50:28
|
LL | let in_call = Box::new(f32::clamp);
| ^^^^^^^^^^

error: use of a disallowed method `regex::Regex::new`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:50:53
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:51:53
|
LL | let in_method_call = ["^", "$"].into_iter().map(Regex::new);
| ^^^^^^^^^^

error: use of a disallowed method `futures::stream::select_all`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:53:31
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:54:31
|
LL | let same_name_as_module = select_all(vec![empty::<()>()]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::local_fn`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:55:5
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:56:5
|
LL | local_fn();
| ^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::local_mod::f`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:56:5
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:57:5
|
LL | local_mod::f();
| ^^^^^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::Struct::method`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:58:5
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:59:5
|
LL | s.method();
| ^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::Trait::provided_method`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:59:5
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:60:5
|
LL | s.provided_method();
| ^^^^^^^^^^^^^^^^^^^

error: use of a disallowed method `conf_disallowed_methods::Trait::implemented_method`
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:60:5
--> tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs:61:5
|
LL | s.implemented_method();
| ^^^^^^^^^^^^^^^^^^^^^^
Expand Down
4 changes: 4 additions & 0 deletions src/tools/clippy/tests/ui/arithmetic_side_effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
unconditional_panic
)]
#![feature(const_mut_refs)]
#![feature(f128)]
#![feature(f16)]
#![warn(clippy::arithmetic_side_effects)]

extern crate proc_macro_derive;
Expand Down Expand Up @@ -162,8 +164,10 @@ pub fn association_with_structures_should_not_trigger_the_lint() {
}

pub fn hard_coded_allowed() {
let _ = 1f16 + 1f16;
let _ = 1f32 + 1f32;
let _ = 1f64 + 1f64;
let _ = 1f128 + 1f128;

let _ = Saturating(0u32) + Saturating(0u32);
let _ = String::new() + "";
Expand Down
Loading

0 comments on commit d3f5e7b

Please sign in to comment.