From 3ab68bad104b176d62352b7366691ce25272fe08 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Tue, 25 Mar 2025 11:57:57 +0100 Subject: [PATCH 1/3] uucore: format: Fix hexadecimal uppercase print (again) When '%A' format is specified, we also need to capitalize the `0x`, i.e. `0XEP-3`, not `0xEP-3`. --- src/uucore/src/lib/features/format/num_format.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/uucore/src/lib/features/format/num_format.rs b/src/uucore/src/lib/features/format/num_format.rs index b636744df60..f89c52102f4 100644 --- a/src/uucore/src/lib/features/format/num_format.rs +++ b/src/uucore/src/lib/features/format/num_format.rs @@ -509,9 +509,9 @@ fn format_float_hexadecimal( ) -> String { debug_assert!(!bd.is_negative()); - let exp_char = match case { - Case::Lowercase => 'p', - Case::Uppercase => 'P', + let (prefix, exp_char) = match case { + Case::Lowercase => ("0x", 'p'), + Case::Uppercase => ("0X", 'P'), }; if BigDecimal::zero().eq(bd) { @@ -607,7 +607,7 @@ fn format_float_hexadecimal( "" }; - format!("0x{first_digit}{dot}{remaining_digits}{exp_char}{exponent:+}") + format!("{prefix}{first_digit}{dot}{remaining_digits}{exp_char}{exponent:+}") } fn strip_fractional_zeroes_and_dot(s: &mut String) { @@ -964,8 +964,8 @@ mod test { ForceDecimal::No, ) }; - assert_eq!(f("0.00001"), "0xA.7C5AC4P-20"); - assert_eq!(f("0.125"), "0x8.000000P-6"); + assert_eq!(f("0.00001"), "0XA.7C5AC4P-20"); + assert_eq!(f("0.125"), "0X8.000000P-6"); // Test "0e10"/"0e-10". From cppreference.com: "If the value is ​0​, the exponent is also ​0​." let f = |digits, scale| { @@ -1178,7 +1178,7 @@ mod test { assert_eq!(f("%e", &(-123.0).into()), "-1.230000e+02"); assert_eq!(f("%#09.e", &(-100.0).into()), "-001.e+02"); assert_eq!(f("%# 9.E", &100.0.into()), " 1.E+02"); - assert_eq!(f("% 12.2A", &(-100.0).into()), " -0xC.80P+3"); + assert_eq!(f("% 12.2A", &(-100.0).into()), " -0XC.80P+3"); } #[test] From 3f24796c8dcfa427875d0b76ba1e71d3abc65b0a Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Tue, 25 Mar 2025 11:18:28 +0100 Subject: [PATCH 2/3] uucore: format: Use Option for Float precision The default precision for float actually depends on the format. It's _usually_ 6, but it's architecture-specific for hexadecimal floats. Set the precision as an Option, so that: - We don't need to sprinkle `6` in callers - We can actually handle unspecified precision correctly in float printing (next change). --- src/uu/seq/src/seq.rs | 2 +- .../src/lib/features/format/num_format.rs | 95 +++++++++++++------ src/uucore/src/lib/features/format/spec.rs | 8 +- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/src/uu/seq/src/seq.rs b/src/uu/seq/src/seq.rs index 827a8335eff..ff63363ec1e 100644 --- a/src/uu/seq/src/seq.rs +++ b/src/uu/seq/src/seq.rs @@ -169,7 +169,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { variant: FloatVariant::Decimal, width: padding, alignment: num_format::NumberAlignment::RightZero, - precision, + precision: Some(precision), ..Default::default() }, // format without precision: hexadecimal floats diff --git a/src/uucore/src/lib/features/format/num_format.rs b/src/uucore/src/lib/features/format/num_format.rs index f89c52102f4..21aa3bbbd1b 100644 --- a/src/uucore/src/lib/features/format/num_format.rs +++ b/src/uucore/src/lib/features/format/num_format.rs @@ -220,7 +220,10 @@ pub struct Float { pub width: usize, pub positive_sign: PositiveSign, pub alignment: NumberAlignment, - pub precision: usize, + // For float, the default precision depends on the format, usually 6, + // but something architecture-specific for %a. Set this to None to + // use the default. + pub precision: Option, } impl Default for Float { @@ -232,7 +235,7 @@ impl Default for Float { width: 0, positive_sign: PositiveSign::None, alignment: NumberAlignment::Left, - precision: 6, + precision: None, } } } @@ -315,8 +318,8 @@ impl Formatter<&ExtendedBigDecimal> for Float { }; let precision = match precision { - Some(CanAsterisk::Fixed(x)) => x, - None => 6, // Default float precision (C standard) + Some(CanAsterisk::Fixed(x)) => Some(x), + None => None, Some(CanAsterisk::Asterisk) => return Err(FormatError::WrongSpecType), }; @@ -360,8 +363,13 @@ fn format_float_non_finite(e: &ExtendedBigDecimal, case: Case) -> String { s } -fn format_float_decimal(bd: &BigDecimal, precision: usize, force_decimal: ForceDecimal) -> String { +fn format_float_decimal( + bd: &BigDecimal, + precision: Option, + force_decimal: ForceDecimal, +) -> String { debug_assert!(!bd.is_negative()); + let precision = precision.unwrap_or(6); // Default %f precision (C standard) if precision == 0 { let (bi, scale) = bd.as_bigint_and_scale(); if scale == 0 && force_decimal != ForceDecimal::Yes { @@ -376,11 +384,12 @@ fn format_float_decimal(bd: &BigDecimal, precision: usize, force_decimal: ForceD fn format_float_scientific( bd: &BigDecimal, - precision: usize, + precision: Option, case: Case, force_decimal: ForceDecimal, ) -> String { debug_assert!(!bd.is_negative()); + let precision = precision.unwrap_or(6); // Default %e precision (C standard) let exp_char = match case { Case::Lowercase => 'e', Case::Uppercase => 'E', @@ -421,11 +430,12 @@ fn format_float_scientific( fn format_float_shortest( bd: &BigDecimal, - precision: usize, + precision: Option, case: Case, force_decimal: ForceDecimal, ) -> String { debug_assert!(!bd.is_negative()); + let precision = precision.unwrap_or(6); // Default %g precision (C standard) // Note: Precision here is how many digits should be displayed in total, // instead of how many digits in the fractional part. @@ -503,11 +513,23 @@ fn format_float_shortest( fn format_float_hexadecimal( bd: &BigDecimal, - precision: usize, + precision: Option, case: Case, force_decimal: ForceDecimal, ) -> String { debug_assert!(!bd.is_negative()); + // Default precision for %a is supposed to be sufficient to represent the + // exact value. This is platform specific, GNU coreutils uses a `long double`, + // which can be equivalent to a f64, f128, or an x86(-64) specific "f80". + // We have arbitrary precision in base 10, so we can't always represent + // the value exactly (e.g. 0.1 is c.ccccc...). + // + // Emulate x86(-64) behavior, where 64 bits are printed in total, that's + // 16 hex digits, including 1 before the decimal point (so 15 after). + // + // TODO: Make this configurable? e.g. arm64 value would be 28 (f128), + // arm value 13 (f64). + let precision = precision.unwrap_or(15); let (prefix, exp_char) = match case { Case::Lowercase => ("0x", 'p'), @@ -706,7 +728,8 @@ mod test { #[test] fn decimal_float() { use super::format_float_decimal; - let f = |x| format_float_decimal(&BigDecimal::from_f64(x).unwrap(), 6, ForceDecimal::No); + let f = + |x| format_float_decimal(&BigDecimal::from_f64(x).unwrap(), Some(6), ForceDecimal::No); assert_eq!(f(0.0), "0.000000"); assert_eq!(f(1.0), "1.000000"); assert_eq!(f(100.0), "100.000000"); @@ -717,11 +740,23 @@ mod test { assert_eq!(f(1.999_999_5), "1.999999"); assert_eq!(f(1.999_999_6), "2.000000"); - let f = |x| format_float_decimal(&BigDecimal::from_f64(x).unwrap(), 0, ForceDecimal::Yes); + let f = |x| { + format_float_decimal( + &BigDecimal::from_f64(x).unwrap(), + Some(0), + ForceDecimal::Yes, + ) + }; assert_eq!(f(100.0), "100."); // Test arbitrary precision: long inputs that would not fit in a f64, print 24 digits after decimal point. - let f = |x| format_float_decimal(&BigDecimal::from_str(x).unwrap(), 24, ForceDecimal::No); + let f = |x| { + format_float_decimal( + &BigDecimal::from_str(x).unwrap(), + Some(24), + ForceDecimal::No, + ) + }; assert_eq!(f("0.12345678901234567890"), "0.123456789012345678900000"); assert_eq!( f("1234567890.12345678901234567890"), @@ -737,7 +772,11 @@ mod test { // TODO: Enable after https://github.com/akubera/bigdecimal-rs/issues/144 is fixed, // as our workaround is in .fmt. let f = |digits, scale| { - format_float_decimal(&BigDecimal::from_bigint(digits, scale), 6, ForceDecimal::No) + format_float_decimal( + &BigDecimal::from_bigint(digits, scale), + Some(6), + ForceDecimal::No, + ) }; assert_eq!(f(0.into(), 0), "0.000000"); assert_eq!(f(0.into(), -10), "0.000000"); @@ -750,7 +789,7 @@ mod test { let f = |x| { format_float_scientific( &BigDecimal::from_f64(x).unwrap(), - 6, + None, Case::Lowercase, ForceDecimal::No, ) @@ -766,7 +805,7 @@ mod test { let f = |x| { format_float_scientific( &BigDecimal::from_f64(x).unwrap(), - 6, + Some(6), Case::Uppercase, ForceDecimal::No, ) @@ -778,7 +817,7 @@ mod test { let f = |digits, scale| { format_float_scientific( &BigDecimal::from_bigint(digits, scale), - 6, + Some(6), Case::Lowercase, ForceDecimal::No, ) @@ -795,7 +834,7 @@ mod test { let f = |x| { format_float_scientific( &BigDecimal::from_f64(x).unwrap(), - 0, + Some(0), Case::Lowercase, ForceDecimal::No, ) @@ -811,7 +850,7 @@ mod test { let f = |x| { format_float_scientific( &BigDecimal::from_f64(x).unwrap(), - 0, + Some(0), Case::Lowercase, ForceDecimal::Yes, ) @@ -831,7 +870,7 @@ mod test { let f = |x| { format_float_shortest( &BigDecimal::from_f64(x).unwrap(), - 6, + None, Case::Lowercase, ForceDecimal::No, ) @@ -853,7 +892,7 @@ mod test { let f = |x| { format_float_shortest( &BigDecimal::from_f64(x).unwrap(), - 6, + None, Case::Lowercase, ForceDecimal::Yes, ) @@ -875,7 +914,7 @@ mod test { let f = |x| { format_float_shortest( &BigDecimal::from_f64(x).unwrap(), - 0, + Some(0), Case::Lowercase, ForceDecimal::No, ) @@ -894,7 +933,7 @@ mod test { let f = |x| { format_float_shortest( &BigDecimal::from_f64(x).unwrap(), - 0, + Some(0), Case::Lowercase, ForceDecimal::Yes, ) @@ -920,7 +959,7 @@ mod test { let f = |x| { format_float_hexadecimal( &BigDecimal::from_str(x).unwrap(), - 6, + Some(6), Case::Lowercase, ForceDecimal::No, ) @@ -935,7 +974,7 @@ mod test { let f = |x| { format_float_hexadecimal( &BigDecimal::from_str(x).unwrap(), - 0, + Some(0), Case::Lowercase, ForceDecimal::No, ) @@ -947,7 +986,7 @@ mod test { let f = |x| { format_float_hexadecimal( &BigDecimal::from_str(x).unwrap(), - 0, + Some(0), Case::Lowercase, ForceDecimal::Yes, ) @@ -959,7 +998,7 @@ mod test { let f = |x| { format_float_hexadecimal( &BigDecimal::from_str(x).unwrap(), - 6, + Some(6), Case::Uppercase, ForceDecimal::No, ) @@ -971,7 +1010,7 @@ mod test { let f = |digits, scale| { format_float_hexadecimal( &BigDecimal::from_bigint(digits, scale), - 6, + Some(6), Case::Lowercase, ForceDecimal::No, ) @@ -1001,7 +1040,7 @@ mod test { let f = |x| { format_float_shortest( &BigDecimal::from_f64(x).unwrap(), - 6, + None, Case::Lowercase, ForceDecimal::No, ) @@ -1019,7 +1058,7 @@ mod test { let f = |x| { format_float_shortest( &BigDecimal::from_f64(x).unwrap(), - 6, + None, Case::Lowercase, ForceDecimal::No, ) diff --git a/src/uucore/src/lib/features/format/spec.rs b/src/uucore/src/lib/features/format/spec.rs index 13025ae7413..e6ac2e88426 100644 --- a/src/uucore/src/lib/features/format/spec.rs +++ b/src/uucore/src/lib/features/format/spec.rs @@ -432,11 +432,13 @@ impl Spec { precision, } => { let width = resolve_asterisk(*width, &mut args).unwrap_or(0); - let precision = resolve_asterisk(*precision, &mut args).unwrap_or(6); + let precision = resolve_asterisk(*precision, &mut args); let f: ExtendedBigDecimal = args.get_extended_big_decimal(); - if precision as u64 > i32::MAX as u64 { - return Err(FormatError::InvalidPrecision(precision.to_string())); + if precision.is_some_and(|p| p as u64 > i32::MAX as u64) { + return Err(FormatError::InvalidPrecision( + precision.unwrap().to_string(), + )); } num_format::Float { From 8cf4da0b19d727a36fc3b90ab027ae22b8eb9ff1 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Tue, 25 Mar 2025 11:18:57 +0100 Subject: [PATCH 3/3] uucore: format: Fix hexadecimal default format print The default hex format, on x86(-64) prints 15 digits after the decimal point, _but_ also trims trailing zeros, so it's not just a simple default precision and a little bit of extra handling is required. Also add a bunch of tests. Fixes #7364. --- .../src/lib/features/format/num_format.rs | 86 ++++++++++++++++--- tests/by-util/test_printf.rs | 2 - 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/uucore/src/lib/features/format/num_format.rs b/src/uucore/src/lib/features/format/num_format.rs index 21aa3bbbd1b..903843958e7 100644 --- a/src/uucore/src/lib/features/format/num_format.rs +++ b/src/uucore/src/lib/features/format/num_format.rs @@ -524,12 +524,15 @@ fn format_float_hexadecimal( // We have arbitrary precision in base 10, so we can't always represent // the value exactly (e.g. 0.1 is c.ccccc...). // - // Emulate x86(-64) behavior, where 64 bits are printed in total, that's - // 16 hex digits, including 1 before the decimal point (so 15 after). + // Note that this is the maximum precision, trailing 0's are trimmed when + // printing. + // + // Emulate x86(-64) behavior, where 64 bits at _most_ are printed in total, + // that's 16 hex digits, including 1 before the decimal point (so 15 after). // // TODO: Make this configurable? e.g. arm64 value would be 28 (f128), // arm value 13 (f64). - let precision = precision.unwrap_or(15); + let max_precision = precision.unwrap_or(15); let (prefix, exp_char) = match case { Case::Lowercase => ("0x", 'p'), @@ -537,10 +540,12 @@ fn format_float_hexadecimal( }; if BigDecimal::zero().eq(bd) { - return if force_decimal == ForceDecimal::Yes && precision == 0 { + // To print 0, we don't ever need any digits after the decimal point, so default to + // that if precision is not specified. + return if force_decimal == ForceDecimal::Yes && precision.unwrap_or(0) == 0 { format!("0x0.{exp_char}+0") } else { - format!("0x{:.*}{exp_char}+0", precision, 0.0) + format!("0x{:.*}{exp_char}+0", precision.unwrap_or(0), 0.0) }; } @@ -575,7 +580,8 @@ fn format_float_hexadecimal( // Then, dividing by 5^-exp10 loses at most -exp10*3 binary digits // (since 5^-exp10 < 8^-exp10), so we add that, and another bit for // rounding. - let margin = ((precision + 1) as i64 * 4 - frac10.bits() as i64).max(0) + -exp10 * 3 + 1; + let margin = + ((max_precision + 1) as i64 * 4 - frac10.bits() as i64).max(0) + -exp10 * 3 + 1; // frac10 * 10^exp10 = frac10 * 2^margin * 10^exp10 * 2^-margin = // (frac10 * 2^margin * 5^exp10) * 2^exp10 * 2^-margin = @@ -590,7 +596,7 @@ fn format_float_hexadecimal( // so the value will always be between 0x8 and 0xf. // TODO: Make this configurable? e.g. arm64 only displays 1 digit. const BEFORE_BITS: usize = 4; - let wanted_bits = (BEFORE_BITS + precision * 4) as u64; + let wanted_bits = (BEFORE_BITS + max_precision * 4) as u64; let bits = frac2.bits(); exp2 += bits as i64 - wanted_bits as i64; @@ -620,18 +626,39 @@ fn format_float_hexadecimal( digits.make_ascii_uppercase(); } let (first_digit, remaining_digits) = digits.split_at(1); - let exponent = exp2 + (4 * precision) as i64; + let exponent = exp2 + (4 * max_precision) as i64; - let dot = - if !remaining_digits.is_empty() || (precision == 0 && ForceDecimal::Yes == force_decimal) { - "." - } else { - "" - }; + let mut remaining_digits = remaining_digits.to_string(); + if precision.is_none() { + // Trim trailing zeros + strip_fractional_zeroes(&mut remaining_digits); + } + + let dot = if !remaining_digits.is_empty() + || (precision.unwrap_or(0) == 0 && ForceDecimal::Yes == force_decimal) + { + "." + } else { + "" + }; format!("{prefix}{first_digit}{dot}{remaining_digits}{exp_char}{exponent:+}") } +fn strip_fractional_zeroes(s: &mut String) { + let mut trim_to = s.len(); + for (pos, c) in s.char_indices().rev() { + if pos + c.len_utf8() == trim_to { + if c == '0' { + trim_to = pos; + } else { + break; + } + } + } + s.truncate(trim_to); +} + fn strip_fractional_zeroes_and_dot(s: &mut String) { let mut trim_to = s.len(); for (pos, c) in s.char_indices().rev() { @@ -995,6 +1022,37 @@ mod test { assert_eq!(f("0.125"), "0x8.p-6"); assert_eq!(f("256.0"), "0x8.p+5"); + // Default precision, maximum 13 digits (x86-64 behavior) + let f = |x| { + format_float_hexadecimal( + &BigDecimal::from_str(x).unwrap(), + None, + Case::Lowercase, + ForceDecimal::No, + ) + }; + assert_eq!(f("0"), "0x0p+0"); + assert_eq!(f("0.00001"), "0xa.7c5ac471b478423p-20"); + assert_eq!(f("0.125"), "0x8p-6"); + assert_eq!(f("4.25"), "0x8.8p-1"); + assert_eq!(f("17.203125"), "0x8.9ap+1"); + assert_eq!(f("256.0"), "0x8p+5"); + assert_eq!(f("1000.01"), "0xf.a00a3d70a3d70a4p+6"); + assert_eq!(f("65536.0"), "0x8p+13"); + + let f = |x| { + format_float_hexadecimal( + &BigDecimal::from_str(x).unwrap(), + None, + Case::Lowercase, + ForceDecimal::Yes, + ) + }; + assert_eq!(f("0"), "0x0.p+0"); + assert_eq!(f("0.125"), "0x8.p-6"); + assert_eq!(f("4.25"), "0x8.8p-1"); + assert_eq!(f("256.0"), "0x8.p+5"); + let f = |x| { format_float_hexadecimal( &BigDecimal::from_str(x).unwrap(), diff --git a/tests/by-util/test_printf.rs b/tests/by-util/test_printf.rs index 60297e2e379..61e14608a4a 100644 --- a/tests/by-util/test_printf.rs +++ b/tests/by-util/test_printf.rs @@ -390,7 +390,6 @@ fn sub_num_sci_negative() { .stdout_only("-1234 is -1.234000e+03"); } -#[cfg_attr(not(feature = "test_unimplemented"), ignore)] #[test] fn sub_num_hex_float_lower() { new_ucmd!() @@ -399,7 +398,6 @@ fn sub_num_hex_float_lower() { .stdout_only("0xep-4"); } -#[cfg_attr(not(feature = "test_unimplemented"), ignore)] #[test] fn sub_num_hex_float_upper() { new_ucmd!()