Skip to content

Commit

Permalink
seq:improvements after review
Browse files Browse the repository at this point in the history
  • Loading branch information
alexs-sh committed Dec 17, 2024
1 parent 8e4bcd8 commit 4d9a86e
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 9 deletions.
48 changes: 40 additions & 8 deletions src/uu/seq/src/floatparse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@ use crate::numberparse::ParseNumberError;
use bigdecimal::BigDecimal;
use num_traits::FromPrimitive;

/// Parse a number from a floating-point hexadecimal exponent notation.
///
/// # Errors
///
/// This function returns an error if:
/// - the input string is not a valid hexadecimal string
/// - the input data can't be interpreted as ['f64'] or ['BigDecimal']
///
/// # Examples
///
/// ```rust,ignore
/// let input = "0x1.4p-2";
/// let expected = 0.3125;
/// match input.parse::<PreciseNumber>().unwrap().number {
/// ExtendedBigDecimal::BigDecimal(bd) => assert_eq!(bd.to_f64().unwrap(),expected),
/// _ => unreachable!()
/// };
/// ```
pub fn parse_hexadecimal_float(s: &str) -> Result<PreciseNumber, ParseNumberError> {
let value = parse_float(s)?;
let number = BigDecimal::from_f64(value).ok_or(ParseNumberError::Float)?;
Expand All @@ -20,6 +38,13 @@ pub fn parse_hexadecimal_float(s: &str) -> Result<PreciseNumber, ParseNumberErro
))
}

/// Parse a floating-point number from a hexadecimal notation.
///
/// # Errors
///
/// This function returns an error if:
/// - the input string is not a hexadecimal string
/// - input data can't be interpreted as ['f64'] and ['BigDecimal']
fn parse_float(s: &str) -> Result<f64, ParseNumberError> {
let mut s = s.trim();

Expand All @@ -34,13 +59,14 @@ fn parse_float(s: &str) -> Result<f64, ParseNumberError> {
1.0
};

// Is HEX?
if s.starts_with("0x") || s.starts_with("0X") {
s = &s[2..];
} else {
// Return error if not a Hex string
if !s.starts_with("0x") && !s.starts_with("0X") {
return Err(ParseNumberError::Float);
}

// Skip Hex prefix
s = &s[2..];

// Read an integer part (if presented)
let length = s.chars().take_while(|c| c.is_ascii_hexdigit()).count();
let integer = u64::from_str_radix(&s[..length], 16).unwrap_or(0);
Expand Down Expand Up @@ -71,10 +97,10 @@ fn parse_float(s: &str) -> Result<f64, ParseNumberError> {
None
};

// Post checks:
// - Both Fractions & Power values can't be none in the same time
// - string should be consumed. Otherwise, it's possible to have garbage symbols after the HEX
// float
// Post-checks:
// - Both 'fractional' and 'power' values cannot be 'None' at the same time.
// - The entire string must be consumed; otherwise, there could be garbage symbols after the Hex float.

if fractional.is_none() && power.is_none() {
return Err(ParseNumberError::Float);
}
Expand All @@ -89,6 +115,11 @@ fn parse_float(s: &str) -> Result<f64, ParseNumberError> {
Ok(total)
}

/// Parse the fractional part in hexadecimal notation.
///
/// # Errors
///
/// This function returns an error if the string is empty or contains characters that are not hex digits.
fn parse_fractional_part(s: &str) -> Result<f64, ParseNumberError> {
if s.is_empty() {
return Err(ParseNumberError::Float);
Expand All @@ -109,6 +140,7 @@ fn parse_fractional_part(s: &str) -> Result<f64, ParseNumberError> {

#[cfg(test)]
mod tests {

use super::parse_hexadecimal_float;
use crate::{numberparse::ParseNumberError, ExtendedBigDecimal};
use num_traits::ToPrimitive;
Expand Down
18 changes: 17 additions & 1 deletion tests/by-util/test_seq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,23 @@ fn test_parse_valid_hexadecimal_float() {
#[ignore]
#[test]
fn test_parse_valid_hexadecimal_float_format_issues() {
// Parsing is OK, but value representation conflicts with the GNU seq.
// These tests detect differences in the representation of floating-point values with GNU seq.
// There are two key areas to investigate:
//
// 1. GNU seq uses long double (80-bit) types for values, while the current implementation
// relies on f64 (64-bit). This can lead to differences due to varying precision. However, it's
// likely not the primary cause, as even double (64-bit) values can differ when compared to
// f64.
//
// 2. GNU seq uses the %Lg format specifier for printing (see the "get_default_format" function
// ). It appears that Rust lacks a direct equivalent for this format. Additionally, %Lg
// can use %f (floating) or %e (scientific) depending on the precision. There also seem to be
// some differences in the behavior of C and Rust when displaying floating-point or scientific
// notation, at least without additional configuration.
//
// It makes sense to begin by experimenting with formats and attempting to replicate
// the printf("%Lg",...) behavior. Another area worth investigating is glibc, as reviewing its
// code may help uncover additional corner cases or test data that could reveal more issues.

// Test output: 4095.953125
// In fact, the 4095.953125 is correct value.
Expand Down

0 comments on commit 4d9a86e

Please sign in to comment.