Skip to content

Commit 6784809

Browse files
committed
Fix {f16,f32,f64,f128}::rem_euclid
The current implementation of `rem_euclid` for floating point numbers violates the invariant, stated in the documentation, that: ```rust a.rem_euclid(b) ~= a - b * a.div_euclid(b) ``` In a 2001 paper[^1], Daan Leijen (who notably later created the Koka programming language) provides the correct formulation of this (and of `div_euclid`) in "Algorithm E": q_E = q_T - I r_E = r_T + I * b where I = if r_T >= 0 then 0 else if b > 0 then 1 else -1 q_T = trunc(a / b) r_T = a - b * q_T a is a dividend, a real number b is a divisor, a real number In section 1.5 of the paper, he gives a proof of correctness. To encode this in Rust, we might think to use `a % b` for `r_T` (remainder of truncated division). After all, we document[^2] that `a % b` is computed as... ```rust a - b * (a / b).trunc() ``` However, as it turns out, we do not currently compute `Rem` in this way, as can be seen trivially with: ```rust let (x, y) = (11f64, 1.1f64); assert_eq!(x - (x / y).trunc() * y, x % y); //~ PANIC ``` For the moment, therefore, we've encoded `r_T` in the literal way. As we know the maxim, from Knuth, to... > Beware of bugs in the above code; I have only proved it correct, not > tried it. ...we have additionally subjected our encoding of this formulation to fuzzing. It seems to hold up against the desired invariants. This is of course a breaking change. But the current implementation is broken, and libs-api has signaled openness to fixing it. [^1]: "Division and Modulus for Computer Scientists", Daan Leijen, 2001, <https://www.microsoft.com/en-us/research/wp-content/uploads/2016/02/divmodnote-letter.pdf> [^2]: https://doc.rust-lang.org/std/ops/trait.Rem.html#impl-Rem-for-f64
1 parent bd36e69 commit 6784809

File tree

4 files changed

+35
-8
lines changed

4 files changed

+35
-8
lines changed

library/std/src/f128.rs

+11-2
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,17 @@ impl f128 {
309309
#[unstable(feature = "f128", issue = "116909")]
310310
#[must_use = "method returns a new number and does not mutate the original value"]
311311
pub fn rem_euclid(self, rhs: f128) -> f128 {
312-
let r = self % rhs;
313-
if r < 0.0 { r + rhs.abs() } else { r }
312+
if rhs.is_infinite() {
313+
return self;
314+
}
315+
// FIXME(#133755): Though `self % rhs` is documented to be
316+
// equivalent to this, it is not, and the distinction matters
317+
// here.
318+
let r = self - rhs * (self / rhs).trunc();
319+
if r < 0.0 {
320+
return if rhs > 0.0 { r + rhs } else { r - rhs };
321+
}
322+
r
314323
}
315324

316325
/// Raises a number to an integer power.

library/std/src/f16.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,14 @@ impl f16 {
309309
#[unstable(feature = "f16", issue = "116909")]
310310
#[must_use = "method returns a new number and does not mutate the original value"]
311311
pub fn rem_euclid(self, rhs: f16) -> f16 {
312-
let r = self % rhs;
313-
if r < 0.0 { r + rhs.abs() } else { r }
312+
// FIXME(#133755): Though `self % rhs` is documented to be
313+
// equivalent to this, it is not, and the distinction matters
314+
// here.
315+
let r = self - rhs * (self / rhs).trunc();
316+
if r < 0.0 {
317+
return if rhs > 0.0 { r + rhs } else { r - rhs };
318+
}
319+
r
314320
}
315321

316322
/// Raises a number to an integer power.

library/std/src/f32.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,14 @@ impl f32 {
285285
#[inline]
286286
#[stable(feature = "euclidean_division", since = "1.38.0")]
287287
pub fn rem_euclid(self, rhs: f32) -> f32 {
288-
let r = self % rhs;
289-
if r < 0.0 { r + rhs.abs() } else { r }
288+
// FIXME(#133755): Though `self % rhs` is documented to be
289+
// equivalent to this, it is not, and the distinction matters
290+
// here.
291+
let r = self - rhs * (self / rhs).trunc();
292+
if r < 0.0 {
293+
return if rhs > 0.0 { r + rhs } else { r - rhs };
294+
}
295+
r
290296
}
291297

292298
/// Raises a number to an integer power.

library/std/src/f64.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,14 @@ impl f64 {
285285
#[inline]
286286
#[stable(feature = "euclidean_division", since = "1.38.0")]
287287
pub fn rem_euclid(self, rhs: f64) -> f64 {
288-
let r = self % rhs;
289-
if r < 0.0 { r + rhs.abs() } else { r }
288+
// FIXME(#133755): Though `self % rhs` is documented to be
289+
// equivalent to this, it is not, and the distinction matters
290+
// here.
291+
let r = self - rhs * (self / rhs).trunc();
292+
if r < 0.0 {
293+
return if rhs > 0.0 { r + rhs } else { r - rhs };
294+
}
295+
r
290296
}
291297

292298
/// Raises a number to an integer power.

0 commit comments

Comments
 (0)