Skip to content

Fix nan print, simplify negative number printing. #7413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 14, 2025
Merged

Fix nan print, simplify negative number printing. #7413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e3872e8
uucore: format: force NaN back to lowercase
drinkcat Mar 7, 2025
b7dcaa3
uucore: format: print absolute value of float, then add sign
drinkcat Mar 7, 2025
92a291b
test: printf: Add nan, inf, negative zero
drinkcat Mar 7, 2025
0c0d119
test: printf: Add a test for scientific printing of negative number
drinkcat Mar 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 32 additions & 36 deletions src/uucore/src/lib/features/format/num_format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl Formatter for SignedInt {
x.abs().to_string()
};

let sign_indicator = get_sign_indicator(self.positive_sign, &x);
let sign_indicator = get_sign_indicator(self.positive_sign, x.is_negative());

write_output(writer, sign_indicator, s, self.width, self.alignment)
}
Expand Down Expand Up @@ -239,8 +239,9 @@ impl Default for Float {
impl Formatter for Float {
type Input = f64;

fn fmt(&self, writer: impl Write, x: Self::Input) -> std::io::Result<()> {
let mut s = if x.is_finite() {
fn fmt(&self, writer: impl Write, f: Self::Input) -> std::io::Result<()> {
let x = f.abs();
let s = if x.is_finite() {
match self.variant {
FloatVariant::Decimal => {
format_float_decimal(x, self.precision, self.force_decimal)
Expand All @@ -259,11 +260,7 @@ impl Formatter for Float {
format_float_non_finite(x, self.case)
};

// The format function will parse `x` together with its sign char,
// which should be placed in `sign_indicator`. So drop it here
s = if x < 0. { s[1..].to_string() } else { s };

let sign_indicator = get_sign_indicator(self.positive_sign, &x);
let sign_indicator = get_sign_indicator(self.positive_sign, f.is_sign_negative());

write_output(writer, sign_indicator, s, self.width, self.alignment)
}
Expand Down Expand Up @@ -309,8 +306,8 @@ impl Formatter for Float {
}
}

fn get_sign_indicator<T: PartialOrd + Default>(sign: PositiveSign, x: &T) -> String {
if *x >= T::default() {
fn get_sign_indicator(sign: PositiveSign, negative: bool) -> String {
if !negative {
match sign {
PositiveSign::None => String::new(),
PositiveSign::Plus => String::from("+"),
Expand All @@ -324,13 +321,15 @@ fn get_sign_indicator<T: PartialOrd + Default>(sign: PositiveSign, x: &T) -> Str
fn format_float_non_finite(f: f64, case: Case) -> String {
debug_assert!(!f.is_finite());
let mut s = format!("{f}");
if case == Case::Uppercase {
s.make_ascii_uppercase();
match case {
Case::Lowercase => s.make_ascii_lowercase(), // Forces NaN back to nan.
Case::Uppercase => s.make_ascii_uppercase(),
}
s
}

fn format_float_decimal(f: f64, precision: usize, force_decimal: ForceDecimal) -> String {
debug_assert!(!f.is_sign_negative());
if precision == 0 && force_decimal == ForceDecimal::Yes {
format!("{f:.0}.")
} else {
Expand All @@ -344,6 +343,7 @@ fn format_float_scientific(
case: Case,
force_decimal: ForceDecimal,
) -> String {
debug_assert!(!f.is_sign_negative());
let exp_char = match case {
Case::Lowercase => 'e',
Case::Uppercase => 'E',
Expand Down Expand Up @@ -383,6 +383,7 @@ fn format_float_shortest(
case: Case,
force_decimal: ForceDecimal,
) -> String {
debug_assert!(!f.is_sign_negative());
// Precision here is about how many digits should be displayed
// instead of how many digits for the fractional part, this means that if
// we pass this to rust's format string, it's always gonna be one less.
Expand Down Expand Up @@ -459,21 +460,21 @@ fn format_float_hexadecimal(
case: Case,
force_decimal: ForceDecimal,
) -> String {
let (sign, first_digit, mantissa, exponent) = if f == 0.0 {
("", 0, 0, 0)
debug_assert!(!f.is_sign_negative());
let (first_digit, mantissa, exponent) = if f == 0.0 {
(0, 0, 0)
} else {
let bits = f.to_bits();
let sign = if (bits >> 63) == 1 { "-" } else { "" };
let exponent_bits = ((bits >> 52) & 0x7ff) as i64;
let exponent = exponent_bits - 1023;
let mantissa = bits & 0xf_ffff_ffff_ffff;
(sign, 1, mantissa, exponent)
(1, mantissa, exponent)
};

let mut s = match (precision, force_decimal) {
(0, ForceDecimal::No) => format!("{sign}0x{first_digit}p{exponent:+}"),
(0, ForceDecimal::Yes) => format!("{sign}0x{first_digit}.p{exponent:+}"),
_ => format!("{sign}0x{first_digit}.{mantissa:0>13x}p{exponent:+}"),
(0, ForceDecimal::No) => format!("0x{first_digit}p{exponent:+}"),
(0, ForceDecimal::Yes) => format!("0x{first_digit}.p{exponent:+}"),
_ => format!("0x{first_digit}.{mantissa:0>13x}p{exponent:+}"),
};

if case == Case::Uppercase {
Expand Down Expand Up @@ -550,6 +551,18 @@ mod test {
assert_eq!(f(8), "010");
}

#[test]
fn non_finite_float() {
use super::format_float_non_finite;
let f = |x| format_float_non_finite(x, Case::Lowercase);
assert_eq!(f(f64::NAN), "nan");
assert_eq!(f(f64::INFINITY), "inf");

let f = |x| format_float_non_finite(x, Case::Uppercase);
assert_eq!(f(f64::NAN), "NAN");
assert_eq!(f(f64::INFINITY), "INF");
}

#[test]
fn decimal_float() {
use super::format_float_decimal;
Expand Down Expand Up @@ -662,22 +675,14 @@ mod test {
assert_eq!(f(0.125), "0x1.0000000000000p-3");
assert_eq!(f(256.0), "0x1.0000000000000p+8");
assert_eq!(f(65536.0), "0x1.0000000000000p+16");
assert_eq!(f(-0.00001), "-0x1.4f8b588e368f1p-17");
assert_eq!(f(-0.125), "-0x1.0000000000000p-3");
assert_eq!(f(-256.0), "-0x1.0000000000000p+8");
assert_eq!(f(-65536.0), "-0x1.0000000000000p+16");

let f = |x| format_float_hexadecimal(x, 0, Case::Lowercase, ForceDecimal::No);
assert_eq!(f(0.125), "0x1p-3");
assert_eq!(f(256.0), "0x1p+8");
assert_eq!(f(-0.125), "-0x1p-3");
assert_eq!(f(-256.0), "-0x1p+8");

let f = |x| format_float_hexadecimal(x, 0, Case::Lowercase, ForceDecimal::Yes);
assert_eq!(f(0.125), "0x1.p-3");
assert_eq!(f(256.0), "0x1.p+8");
assert_eq!(f(-0.125), "-0x1.p-3");
assert_eq!(f(-256.0), "-0x1.p+8");
}

#[test]
Expand All @@ -703,11 +708,6 @@ mod test {
assert_eq!(f(0.001171875), "0.00117187");
assert_eq!(f(0.0001171875), "0.000117187");
assert_eq!(f(0.001171875001), "0.00117188");
assert_eq!(f(-0.1171875), "-0.117188");
assert_eq!(f(-0.01171875), "-0.0117188");
assert_eq!(f(-0.001171875), "-0.00117187");
assert_eq!(f(-0.0001171875), "-0.000117187");
assert_eq!(f(-0.001171875001), "-0.00117188");
}

#[test]
Expand All @@ -718,9 +718,5 @@ mod test {
assert_eq!(f(0.0001), "0.0001");
assert_eq!(f(0.00001), "1e-05");
assert_eq!(f(0.000001), "1e-06");
assert_eq!(f(-0.001), "-0.001");
assert_eq!(f(-0.0001), "-0.0001");
assert_eq!(f(-0.00001), "-1e-05");
assert_eq!(f(-0.000001), "-1e-06");
}
}
32 changes: 32 additions & 0 deletions tests/by-util/test_printf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,14 @@ fn sub_num_dec_trunc() {
.stdout_only("pi is ~ 3.14159");
}

#[test]
fn sub_num_sci_negative() {
new_ucmd!()
.args(&["-1234 is %e", "-1234"])
.succeeds()
.stdout_only("-1234 is -1.234000e+03");
}

#[cfg_attr(not(feature = "test_unimplemented"), ignore)]
#[test]
fn sub_num_hex_float_lower() {
Expand Down Expand Up @@ -886,6 +894,30 @@ fn float_with_zero_precision_should_pad() {
.stdout_only("-01");
}

#[test]
fn float_non_finite() {
new_ucmd!()
.args(&[
"%f %f %F %f %f %F",
"nan",
"-nan",
"nan",
"inf",
"-inf",
"inf",
])
.succeeds()
.stdout_only("nan -nan NAN inf -inf INF");
}

#[test]
fn float_zero_neg_zero() {
new_ucmd!()
.args(&["%f %f", "0.0", "-0.0"])
.succeeds()
.stdout_only("0.000000 -0.000000");
}

#[test]
fn precision_check() {
new_ucmd!()
Expand Down
Loading