Skip to content

Commit

Permalink
Rollup merge of rust-lang#86479 - exphp-forks:float-debug-exponential…
Browse files Browse the repository at this point in the history
…, r=yaahc

Automatic exponential formatting in Debug

Context: See [this comment from the libs team](rust-lang/rfcs#2729 (comment))

---

Makes `"{:?}"` switch to exponential for floats based on magnitude. The libs team suggested exploring this idea in the discussion thread for RFC rust-lang/rfcs#2729. (**note:** this is **not** an implementation of the RFC; it is an implementation of one of the alternatives)

Thresholds chosen were 1e-4 and 1e16.  Justification described [here](rust-lang/rfcs#2729 (comment)).

**This will require a crater run.**

---

As mentioned in the commit message of 8731d4d, this behavior will not apply when a precision is supplied, because I wanted to preserve the following existing and useful behavior of `{:.PREC?}` (which recursively applies `{:.PREC}` to floats in a struct):

```rust
assert_eq!(
    format!("{:.2?}", [100.0, 0.000004]),
    "[100.00, 0.00]",
)
```

I looked around and am not sure where there are any tests that actually use this in the test suite, though?

All things considered, I'm surprised that this change did not seem to break even a single existing test in `x.py test --stage 2`.  (even when I tried a smaller threshold of 1e6)
  • Loading branch information
JohnTitor authored Oct 19, 2021
2 parents 1af55d1 + 8731d4d commit ca6798a
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 6 deletions.
53 changes: 49 additions & 4 deletions library/core/src/fmt/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,26 @@ use crate::mem::MaybeUninit;
use crate::num::flt2dec;
use crate::num::fmt as numfmt;

#[doc(hidden)]
trait GeneralFormat: PartialOrd {
/// Determines if a value should use exponential based on its magnitude, given the precondition
/// that it will not be rounded any further before it is displayed.
fn already_rounded_value_should_use_exponential(&self) -> bool;
}

macro_rules! impl_general_format {
($($t:ident)*) => {
$(impl GeneralFormat for $t {
fn already_rounded_value_should_use_exponential(&self) -> bool {
let abs = $t::abs_private(*self);
(abs != 0.0 && abs < 1e-4) || abs >= 1e+16
}
})*
}
}

impl_general_format! { f32 f64 }

// Don't inline this so callers don't use the stack space this function
// requires unless they have to.
#[inline(never)]
Expand Down Expand Up @@ -54,8 +74,7 @@ where
fmt.pad_formatted_parts(&formatted)
}

// Common code of floating point Debug and Display.
fn float_to_decimal_common<T>(fmt: &mut Formatter<'_>, num: &T, min_precision: usize) -> Result
fn float_to_decimal_display<T>(fmt: &mut Formatter<'_>, num: &T) -> Result
where
T: flt2dec::DecodableFloat,
{
Expand All @@ -68,6 +87,7 @@ where
if let Some(precision) = fmt.precision {
float_to_decimal_common_exact(fmt, num, sign, precision)
} else {
let min_precision = 0;
float_to_decimal_common_shortest(fmt, num, sign, min_precision)
}
}
Expand Down Expand Up @@ -145,19 +165,44 @@ where
}
}

fn float_to_general_debug<T>(fmt: &mut Formatter<'_>, num: &T) -> Result
where
T: flt2dec::DecodableFloat + GeneralFormat,
{
let force_sign = fmt.sign_plus();
let sign = match force_sign {
false => flt2dec::Sign::Minus,
true => flt2dec::Sign::MinusPlus,
};

if let Some(precision) = fmt.precision {
// this behavior of {:.PREC?} predates exponential formatting for {:?}
float_to_decimal_common_exact(fmt, num, sign, precision)
} else {
// since there is no precision, there will be no rounding
if num.already_rounded_value_should_use_exponential() {
let upper = false;
float_to_exponential_common_shortest(fmt, num, sign, upper)
} else {
let min_precision = 1;
float_to_decimal_common_shortest(fmt, num, sign, min_precision)
}
}
}

macro_rules! floating {
($ty:ident) => {
#[stable(feature = "rust1", since = "1.0.0")]
impl Debug for $ty {
fn fmt(&self, fmt: &mut Formatter<'_>) -> Result {
float_to_decimal_common(fmt, self, 1)
float_to_general_debug(fmt, self)
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl Display for $ty {
fn fmt(&self, fmt: &mut Formatter<'_>) -> Result {
float_to_decimal_common(fmt, self, 0)
float_to_decimal_display(fmt, self)
}
}

Expand Down
2 changes: 1 addition & 1 deletion library/core/src/num/f32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ impl f32 {
// private use internally.
#[inline]
#[rustc_const_unstable(feature = "const_float_classify", issue = "72505")]
const fn abs_private(self) -> f32 {
pub(crate) const fn abs_private(self) -> f32 {
f32::from_bits(self.to_bits() & 0x7fff_ffff)
}

Expand Down
2 changes: 1 addition & 1 deletion library/core/src/num/f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ impl f64 {
// private use internally.
#[inline]
#[rustc_const_unstable(feature = "const_float_classify", issue = "72505")]
const fn abs_private(self) -> f64 {
pub(crate) const fn abs_private(self) -> f64 {
f64::from_bits(self.to_bits() & 0x7fff_ffff_ffff_ffff)
}

Expand Down
24 changes: 24 additions & 0 deletions library/core/tests/fmt/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ fn test_format_f64() {
assert_eq!("1.23456789E3", format!("{:E}", 1234.56789f64));
assert_eq!("0.0", format!("{:?}", 0.0f64));
assert_eq!("1.01", format!("{:?}", 1.01f64));

let high_cutoff = 1e16_f64;
assert_eq!("1e16", format!("{:?}", high_cutoff));
assert_eq!("-1e16", format!("{:?}", -high_cutoff));
assert!(!is_exponential(&format!("{:?}", high_cutoff * (1.0 - 2.0 * f64::EPSILON))));
assert_eq!("-3.0", format!("{:?}", -3f64));
assert_eq!("0.0001", format!("{:?}", 0.0001f64));
assert_eq!("9e-5", format!("{:?}", 0.00009f64));
assert_eq!("1234567.9", format!("{:.1?}", 1234567.89f64));
assert_eq!("1234.6", format!("{:.1?}", 1234.56789f64));
}

#[test]
Expand All @@ -28,4 +38,18 @@ fn test_format_f32() {
assert_eq!("1.2345679E3", format!("{:E}", 1234.56789f32));
assert_eq!("0.0", format!("{:?}", 0.0f32));
assert_eq!("1.01", format!("{:?}", 1.01f32));

let high_cutoff = 1e16_f32;
assert_eq!("1e16", format!("{:?}", high_cutoff));
assert_eq!("-1e16", format!("{:?}", -high_cutoff));
assert!(!is_exponential(&format!("{:?}", high_cutoff * (1.0 - 2.0 * f32::EPSILON))));
assert_eq!("-3.0", format!("{:?}", -3f32));
assert_eq!("0.0001", format!("{:?}", 0.0001f32));
assert_eq!("9e-5", format!("{:?}", 0.00009f32));
assert_eq!("1234567.9", format!("{:.1?}", 1234567.89f32));
assert_eq!("1234.6", format!("{:.1?}", 1234.56789f32));
}

fn is_exponential(s: &str) -> bool {
s.contains("e") || s.contains("E")
}

0 comments on commit ca6798a

Please sign in to comment.