From: Emilie Burgun <emilie@shadamethyst.xyz>
Date: Fri, 17 Jan 2025 19:11:05 +0000 (+0100)
Subject: Fix rnd_i clipping floats that don't fit in Fixnum
X-Git-Tag: v0.10.0~77^2~5
X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=9420c7e41eb88e9918d82657e956088616f258d9;p=scryer-prolog.git

Fix rnd_i clipping floats that don't fit in Fixnum

Fixes #2772.

The current implementation of `rnd_i` incorrectly casts `f` (an `f64`)
into an `i64`, before casting it into an `Integer`.

This fixes that issue by using `Integer::try_from(f)` instead,
and failing if `f` is infinite or NaN.

A fixme is left for a future PR to properly handle the resulting errors
in floor/1 and friends (right now they can only be triggered through FFI).
---

diff --git a/src/arithmetic.rs b/src/arithmetic.rs
index 59a4f4cd..e5d6b114 100644
--- a/src/arithmetic.rs
+++ b/src/arithmetic.rs
@@ -354,17 +354,17 @@ impl<'a> ArithmeticEvaluator<'a> {
 }
 
 // integer division rounding function -- 9.1.3.1.
-pub(crate) fn rnd_i(n: &'_ Number, arena: &mut Arena) -> Number {
+pub(crate) fn rnd_i(n: &'_ Number, arena: &mut Arena) -> Result<Number, EvalError> {
     match n {
         &Number::Integer(i) => {
             let result = (&*i).try_into();
             if let Ok(value) = result {
-                fixnum!(Number, value, arena)
+                Ok(fixnum!(Number, value, arena))
             } else {
-                *n
+                Ok(*n)
             }
         }
-        Number::Fixnum(_) => *n,
+        Number::Fixnum(_) => Ok(*n),
         &Number::Float(f) => {
             let f = f.floor();
 
@@ -372,18 +372,23 @@ pub(crate) fn rnd_i(n: &'_ Number, arena: &mut Arena) -> Number {
             const I64_MAX_TO_F: OrderedFloat<f64> = OrderedFloat(i64::MAX as f64);
 
             if I64_MIN_TO_F <= f && f <= I64_MAX_TO_F {
-                fixnum!(Number, f.into_inner() as i64, arena)
+                Ok(fixnum!(Number, f.into_inner() as i64, arena))
             } else {
-                Number::Integer(arena_alloc!(Integer::from(f.0 as i64), arena))
+                Ok(Number::Integer(arena_alloc!(
+                    Integer::try_from(classify_float(f.0)?).unwrap_or_else(|_| {
+                        unreachable!();
+                    }),
+                    arena
+                )))
             }
         }
         Number::Rational(ref r) => {
-            let (_, floor) = (r.fract(), r.floor());
+            let floor = r.floor();
 
             if let Ok(value) = (&floor).try_into() {
-                fixnum!(Number, value, arena)
+                Ok(fixnum!(Number, value, arena))
             } else {
-                Number::Integer(arena_alloc!(floor, arena))
+                Ok(Number::Integer(arena_alloc!(floor, arena)))
             }
         }
     }
diff --git a/src/machine/arithmetic_ops.rs b/src/machine/arithmetic_ops.rs
index 64e66f4d..290ccc37 100644
--- a/src/machine/arithmetic_ops.rs
+++ b/src/machine/arithmetic_ops.rs
@@ -1044,7 +1044,12 @@ pub(crate) fn sqrt(n1: Number) -> Result<f64, MachineStubGen> {
 
 #[inline]
 pub(crate) fn floor(n1: Number, arena: &mut Arena) -> Number {
-    rnd_i(&n1, arena)
+    rnd_i(&n1, arena).unwrap_or_else(|_err| {
+        // FIXME: Currently floor/1 (and several call sites) are infallible,
+        // but the failing cases (when `n1` is `Number::Float(NAN)` or `Number::Float(INFINITY)`)
+        // are not reachable with standard is/2 operations.
+        todo!("Make floor/1 fallible");
+    })
 }
 
 #[inline]