From cf58a7ce90c21093e1f390ab899d29932d17d42c Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 30 Dec 2024 05:55:41 +0000 Subject: [PATCH] Remove lossy casting in `logspace` Currently `logspace` does a lossy cast from `F::Int` to `usize`. This could be problematic in the rare cases that this is called with a step count exceeding what is representable in `usize`. Resolve this by instead adding bounds so the float's integer type itself can be iterated. --- .../libm/crates/libm-test/src/gen/domain_logspace.rs | 3 +++ .../compiler-builtins/libm/crates/libm-test/src/num.rs | 10 +++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/library/compiler-builtins/libm/crates/libm-test/src/gen/domain_logspace.rs b/library/compiler-builtins/libm/crates/libm-test/src/gen/domain_logspace.rs index 3e69bee34dde..5e37170fa1a4 100644 --- a/library/compiler-builtins/libm/crates/libm-test/src/gen/domain_logspace.rs +++ b/library/compiler-builtins/libm/crates/libm-test/src/gen/domain_logspace.rs @@ -1,5 +1,7 @@ //! A generator that produces logarithmically spaced values within domain bounds. +use std::ops::RangeInclusive; + use libm::support::{IntTy, MinInt}; use crate::domain::HasDomain; @@ -34,6 +36,7 @@ pub fn get_test_cases(_ctx: &CheckCtx) -> impl Iterator where Op: MathOp + HasDomain, IntTy: TryFrom, + RangeInclusive>: Iterator, { let domain = Op::DOMAIN; let start = domain.range_start(); diff --git a/library/compiler-builtins/libm/crates/libm-test/src/num.rs b/library/compiler-builtins/libm/crates/libm-test/src/num.rs index 4aa7f61b05a2..eff2fbc1fdd2 100644 --- a/library/compiler-builtins/libm/crates/libm-test/src/num.rs +++ b/library/compiler-builtins/libm/crates/libm-test/src/num.rs @@ -1,8 +1,9 @@ //! Helpful numeric operations. use std::cmp::min; +use std::ops::RangeInclusive; -use libm::support::{CastInto, Float}; +use libm::support::Float; use crate::{Int, MinInt}; @@ -214,7 +215,10 @@ fn as_ulp_steps(x: F) -> Option { /// to logarithmic spacing of their values. /// /// Note that this tends to skip negative zero, so that needs to be checked explicitly. -pub fn logspace(start: F, end: F, steps: F::Int) -> impl Iterator { +pub fn logspace(start: F, end: F, steps: F::Int) -> impl Iterator +where + RangeInclusive: Iterator, +{ assert!(!start.is_nan()); assert!(!end.is_nan()); assert!(end >= start); @@ -225,7 +229,7 @@ pub fn logspace(start: F, end: F, steps: F::Int) -> impl Iterator