Skip to content
102 changes: 83 additions & 19 deletions src/uu/numfmt/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use uucore::display::Quotable;
use uucore::i18n::decimal::locale_grouping_separator;
use uucore::translate;

use crate::numeric::ParsedNumber;
use crate::options::{NumfmtOptions, RoundMethod, TransformOptions};
use crate::units::{
DisplayableSuffix, RawSuffix, Result, Suffix, Unit, iec_bases_f64, si_bases_f64,
Expand Down Expand Up @@ -147,12 +148,17 @@ fn detailed_error_message(s: &str, unit: Unit, unit_separator: &str) -> Option<S
None
}

fn parse_number_part(s: &str, input: &str) -> Result<f64> {
fn parse_number_part(s: &str, input: &str) -> Result<ParsedNumber> {
if s.ends_with('.') {
return Err(translate!("numfmt-error-invalid-number", "input" => input.quote()));
}

if let Ok(n) = s.parse::<i128>() {
return Ok(ParsedNumber::ExactInt(n));
}

s.parse::<f64>()
.map(ParsedNumber::Float)
.map_err(|_| translate!("numfmt-error-invalid-number", "input" => input.quote()))
}

Expand All @@ -161,7 +167,7 @@ fn parse_suffix(
unit: Unit,
unit_separator: &str,
explicit_unit_separator: bool,
) -> Result<(f64, Option<Suffix>)> {
) -> Result<(ParsedNumber, Option<Suffix>)> {
let trimmed = s.trim_end();
if trimmed.is_empty() {
return Err(translate!("numfmt-error-invalid-number-empty"));
Expand Down Expand Up @@ -364,7 +370,22 @@ fn remove_suffix(i: f64, s: Option<Suffix>, u: Unit) -> Result<f64> {
}
}

fn transform_from(s: &str, opts: &TransformOptions, options: &NumfmtOptions) -> Result<f64> {
fn try_scale_exact_int_with_from_unit(
value: ParsedNumber,
from_unit: usize,
) -> Option<ParsedNumber> {
let integer = value.exact_int()?;
let from_unit = i128::try_from(from_unit).ok()?;
let scaled = integer.checked_mul(from_unit)?;

Some(ParsedNumber::ExactInt(scaled))
}

fn transform_from(
s: &str,
opts: &TransformOptions,
options: &NumfmtOptions,
) -> Result<ParsedNumber> {
let (i, suffix) = parse_suffix(
s,
opts.from,
Expand All @@ -375,17 +396,24 @@ fn transform_from(s: &str, opts: &TransformOptions, options: &NumfmtOptions) ->
detailed_error_message(s, opts.from, &options.unit_separator).unwrap_or(original)
})?;
let had_no_suffix = suffix.is_none();
let i = i * (opts.from_unit as f64);

if had_no_suffix {
if let Some(scaled) = try_scale_exact_int_with_from_unit(i, opts.from_unit) {
return Ok(scaled);
}
}

let i = i.to_f64() * (opts.from_unit as f64);

remove_suffix(i, suffix, opts.from).map(|n| {
// GNU numfmt doesn't round values if no --from argument is provided by the user
if opts.from == Unit::None || had_no_suffix {
let n = if opts.from == Unit::None || had_no_suffix {
if n == -0.0 { 0.0 } else { n }
} else if n < 0.0 {
-n.abs().ceil()
} else {
n.ceil()
}
};
ParsedNumber::Float(n)
})
}

Expand Down Expand Up @@ -480,13 +508,43 @@ fn consider_suffix(
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A detail: I would add an empty line as a separator between the two functions.

Suggested change
}
}


fn try_format_exact_int_without_suffix_scaling(
value: ParsedNumber,
opts: &TransformOptions,
precision: usize,
) -> Option<String> {
if opts.to != Unit::None {
return None;
}

let integer = value.exact_int()?;
let to_unit = i128::try_from(opts.to_unit).ok()?;

if integer % to_unit != 0 {
return None;
}

let scaled = integer / to_unit;

Some(if precision == 0 {
scaled.to_string()
} else {
format!("{scaled}.{}", "0".repeat(precision))
})
}

fn transform_to(
s: f64,
s: ParsedNumber,
opts: &TransformOptions,
round_method: RoundMethod,
precision: usize,
unit_separator: &str,
) -> Result<String> {
if let Some(result) = try_format_exact_int_without_suffix_scaling(s, opts, precision) {
return Ok(result);
}

let s = s.to_f64();
let i2 = s / (opts.to_unit as f64);
let (i2, s) = consider_suffix(i2, opts.to, round_method, precision)?;
Ok(match s {
Expand Down Expand Up @@ -771,7 +829,7 @@ mod tests {
let result = parse_suffix("1Q", Unit::Auto, "", false);
assert!(result.is_ok());
let (number, suffix) = result.unwrap();
assert_eq!(number, 1.0);
assert_eq!(number.to_f64(), 1.0);
assert!(suffix.is_some());
let (raw_suffix, with_i) = suffix.unwrap();
assert_eq!(raw_suffix as i32, RawSuffix::Q as i32);
Expand All @@ -780,7 +838,7 @@ mod tests {
let result = parse_suffix("2R", Unit::Auto, "", false);
assert!(result.is_ok());
let (number, suffix) = result.unwrap();
assert_eq!(number, 2.0);
assert_eq!(number.to_f64(), 2.0);
assert!(suffix.is_some());
let (raw_suffix, with_i) = suffix.unwrap();
assert_eq!(raw_suffix as i32, RawSuffix::R as i32);
Expand All @@ -789,7 +847,7 @@ mod tests {
let result = parse_suffix("3k", Unit::Auto, "", false);
assert!(result.is_ok());
let (number, suffix) = result.unwrap();
assert_eq!(number, 3.0);
assert_eq!(number.to_f64(), 3.0);
assert!(suffix.is_some());
let (raw_suffix, with_i) = suffix.unwrap();
assert_eq!(raw_suffix as i32, RawSuffix::K as i32);
Expand All @@ -798,7 +856,7 @@ mod tests {
let result = parse_suffix("4Qi", Unit::Auto, "", false);
assert!(result.is_ok());
let (number, suffix) = result.unwrap();
assert_eq!(number, 4.0);
assert_eq!(number.to_f64(), 4.0);
assert!(suffix.is_some());
let (raw_suffix, with_i) = suffix.unwrap();
assert_eq!(raw_suffix as i32, RawSuffix::Q as i32);
Expand All @@ -807,7 +865,7 @@ mod tests {
let result = parse_suffix("5Ri", Unit::Auto, "", false);
assert!(result.is_ok());
let (number, suffix) = result.unwrap();
assert_eq!(number, 5.0);
assert_eq!(number.to_f64(), 5.0);
assert!(suffix.is_some());
let (raw_suffix, with_i) = suffix.unwrap();
assert_eq!(raw_suffix as i32, RawSuffix::R as i32);
Expand Down Expand Up @@ -1023,9 +1081,9 @@ mod tests {

#[test]
fn test_parse_number_part_valid() {
assert_eq!(parse_number_part("42", "42").unwrap(), 42.0);
assert_eq!(parse_number_part("-3.5", "-3.5").unwrap(), -3.5);
assert_eq!(parse_number_part("0", "0").unwrap(), 0.0);
assert_eq!(parse_number_part("42", "42").unwrap().to_f64(), 42.0);
assert_eq!(parse_number_part("-3.5", "-3.5").unwrap().to_f64(), -3.5);
assert_eq!(parse_number_part("0", "0").unwrap().to_f64(), 0.0);
}

#[test]
Expand Down Expand Up @@ -1094,15 +1152,21 @@ mod tests {
#[test]
fn test_parse_number_part_large_and_tiny() {
assert_eq!(
parse_number_part("999999999999", "999999999999").unwrap(),
parse_number_part("999999999999", "999999999999")
.unwrap()
.to_f64(),
999_999_999_999.0
);
assert_eq!(
parse_number_part("0.000000001", "0.000000001").unwrap(),
parse_number_part("0.000000001", "0.000000001")
.unwrap()
.to_f64(),
0.000_000_001
);
assert_eq!(
parse_number_part("-99999999", "-99999999").unwrap(),
parse_number_part("-99999999", "-99999999")
.unwrap()
.to_f64(),
-99_999_999.0
);
}
Expand Down
30 changes: 30 additions & 0 deletions src/uu/numfmt/src/numeric.rs
Comment thread
cakebaker marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// This file is part of the uutils coreutils package.
//
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.

// This file is written to solve #11654
// This mod is to preserve numeric precision for large integers.
// GNU numfmt has a better precision on floats due to 'long double'.

#[derive(Clone, Copy, Debug, PartialEq)]
pub(crate) enum ParsedNumber {
ExactInt(i128),
Float(f64),
}

impl ParsedNumber {
pub(crate) fn to_f64(self) -> f64 {
match self {
Self::ExactInt(n) => n as f64,
Self::Float(n) => n,
}
}

pub(crate) fn exact_int(self) -> Option<i128> {
match self {
Self::ExactInt(n) => Some(n),
Self::Float(_) => None,
}
}
}
2 changes: 2 additions & 0 deletions src/uu/numfmt/src/numfmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use uucore::{format_usage, os_str_as_bytes, show, translate};
pub mod errors;
pub mod format;
pub mod options;

mod numeric;
mod units;

/// Format a single line and write it, handling `--invalid` error modes.
Expand Down
1 change: 0 additions & 1 deletion tests/by-util/test_numfmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,6 @@ fn test_negative_number_with_double_dash_gnu_compat_issue_11653() {
// https://github.com/uutils/coreutils/issues/11654
// uutils parses large integers through f64, losing precision past 2^53.
#[test]
#[ignore = "GNU compat: see uutils/coreutils#11654"]
fn test_large_integer_precision_loss_issue_11654() {
new_ucmd!()
.args(&["--from=iec", "9153396227555392131"])
Expand Down
Loading