From 01ded5898b1863e195856d2678323315ae224463 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Sun, 2 Nov 2014 22:53:48 +1100 Subject: [PATCH] Simplify float string conversion function further We now have a really simple function signature: pub fn from_str_radix_float(src: &str, radix: uint) -> Option By removing some of the arguments, we remove the possibility of some invalid states. --- src/libstd/num/f32.rs | 12 +- src/libstd/num/f64.rs | 8 +- src/libstd/num/strconv.rs | 256 ++++++++++++++++---------------------- 3 files changed, 115 insertions(+), 161 deletions(-) diff --git a/src/libstd/num/f32.rs b/src/libstd/num/f32.rs index 448cb2d01fe2c..ebb1aee816e24 100644 --- a/src/libstd/num/f32.rs +++ b/src/libstd/num/f32.rs @@ -359,8 +359,8 @@ pub fn to_str_exp_digits(num: f32, dig: uint, upper: bool) -> String { /// `None` if the string did not represent a valid number. Otherwise, /// `Some(n)` where `n` is the floating-point number represented by `[num]`. #[inline] -pub fn from_str_hex(num: &str) -> Option { - strconv::from_str_float(num, 16u, true, strconv::ExpBin) +pub fn from_str_hex(src: &str) -> Option { + strconv::from_str_radix_float(src, 16u) } impl FromStr for f32 { @@ -390,8 +390,8 @@ impl FromStr for f32 { /// `None` if the string did not represent a valid number. Otherwise, /// `Some(n)` where `n` is the floating-point number represented by `num`. #[inline] - fn from_str(val: &str) -> Option { - strconv::from_str_float(val, 10u, true, strconv::ExpDec) + fn from_str(src: &str) -> Option { + strconv::from_str_radix_float(src, 10u) } } @@ -414,8 +414,8 @@ impl num::FromStrRadix for f32 { /// `None` if the string did not represent a valid number. Otherwise, /// `Some(n)` where `n` is the floating-point number represented by `num`. #[inline] - fn from_str_radix(val: &str, rdx: uint) -> Option { - strconv::from_str_float(val, rdx, false, strconv::ExpNone) + fn from_str_radix(src: &str, radix: uint) -> Option { + strconv::from_str_radix_float(src, radix) } } diff --git a/src/libstd/num/f64.rs b/src/libstd/num/f64.rs index f49e14cb04bab..e1a46f5710f50 100644 --- a/src/libstd/num/f64.rs +++ b/src/libstd/num/f64.rs @@ -368,7 +368,7 @@ pub fn to_str_exp_digits(num: f64, dig: uint, upper: bool) -> String { /// `Some(n)` where `n` is the floating-point number represented by `[num]`. #[inline] pub fn from_str_hex(num: &str) -> Option { - strconv::from_str_float(num, 16u, true, strconv::ExpBin) + strconv::from_str_radix_float(num, 16u) } impl FromStr for f64 { @@ -399,7 +399,7 @@ impl FromStr for f64 { /// `Some(n)` where `n` is the floating-point number represented by `num`. #[inline] fn from_str(val: &str) -> Option { - strconv::from_str_float(val, 10u, true, strconv::ExpDec) + strconv::from_str_radix_float(val, 10u) } } @@ -422,8 +422,8 @@ impl num::FromStrRadix for f64 { /// `None` if the string did not represent a valid number. Otherwise, /// `Some(n)` where `n` is the floating-point number represented by `num`. #[inline] - fn from_str_radix(val: &str, rdx: uint) -> Option { - strconv::from_str_float(val, rdx, false, strconv::ExpNone) + fn from_str_radix(val: &str, radix: uint) -> Option { + strconv::from_str_radix_float(val, radix) } } diff --git a/src/libstd/num/strconv.rs b/src/libstd/num/strconv.rs index 0e8d5bc5ba22e..7a02d8d77b0b1 100644 --- a/src/libstd/num/strconv.rs +++ b/src/libstd/num/strconv.rs @@ -424,69 +424,18 @@ pub fn float_to_str_common( // Some constants for from_str_bytes_common's input validation, // they define minimum radix values for which the character is a valid digit. static DIGIT_P_RADIX: uint = ('p' as uint) - ('a' as uint) + 11u; -static DIGIT_I_RADIX: uint = ('i' as uint) - ('a' as uint) + 11u; static DIGIT_E_RADIX: uint = ('e' as uint) - ('a' as uint) + 11u; -/** - * Parses a string as a number. This is meant to - * be a common base implementation for all numeric string conversion - * functions like `from_str()` or `from_str_radix()`. - * - * # Arguments - * - `src` - The string to parse. - * - `radix` - Which base to parse the number as. Accepts 2-36. - * - `special` - Whether to accept special values like `inf` - * and `NaN`. Can conflict with `radix`, see Failure. - * - `exponent` - Which exponent format to accept. Options are: - * - `ExpNone`: No Exponent, accepts just plain numbers like `42` or - * `-8.2`. - * - `ExpDec`: Accepts numbers with a decimal exponent like `42e5` or - * `8.2E-2`. The exponent string itself is always base 10. - * Can conflict with `radix`, see Failure. - * - `ExpBin`: Accepts numbers with a binary exponent like `42P-8` or - * `FFp128`. The exponent string itself is always base 10. - * Can conflict with `radix`, see Failure. - * - * # Return value - * Returns `Some(n)` if `buf` parses to a number n without overflowing, and - * `None` otherwise, depending on the constraints set by the remaining - * arguments. - * - * # Failure - * - Fails if `radix` < 2 or `radix` > 36. - * - Fails if `radix` > 14 and `exponent` is `ExpDec` due to conflict - * between digit and exponent sign `'e'`. - * - Fails if `radix` > 25 and `exponent` is `ExpBin` due to conflict - * between digit and exponent sign `'p'`. - * - Fails if `radix` > 18 and `special == true` due to conflict - * between digit and lowest first character in `inf` and `NaN`, the `'i'`. - */ -pub fn from_str_float( - src: &str, radix: uint, special: bool, exponent: ExponentFormat, - ) -> Option { - match exponent { - ExpDec if radix >= DIGIT_E_RADIX // decimal exponent 'e' - => panic!("from_str_bytes_common: radix {} incompatible with \ - use of 'e' as decimal exponent", radix), - ExpBin if radix >= DIGIT_P_RADIX // binary exponent 'p' - => panic!("from_str_bytes_common: radix {} incompatible with \ - use of 'p' as binary exponent", radix), - _ if special && radix >= DIGIT_I_RADIX // first digit of 'inf' - => panic!("from_str_bytes_common: radix {} incompatible with \ - special values 'inf' and 'NaN'", radix), - _ if (radix as int) < 2 - => panic!("from_str_bytes_common: radix {} to low, \ - must lie in the range [2, 36]", radix), - _ if (radix as int) > 36 - => panic!("from_str_bytes_common: radix {} to high, \ - must lie in the range [2, 36]", radix), - _ => () - } +pub fn from_str_radix_float(src: &str, radix: uint) -> Option { + assert!(radix >= 2 && radix <= 36, + "from_str_radix_float: must lie in the range `[2, 36]` - found {}", + radix); let _0: T = num::zero(); let _1: T = num::one(); - let radix_gen: T = num::cast(radix as int).unwrap(); + let radix_t: T = num::cast(radix as int).unwrap(); + // Special values match src { "inf" => return Some(Float::infinity()), "-inf" => return Some(Float::neg_infinity()), @@ -501,88 +450,89 @@ pub fn from_str_float( (Some(_), _) => (true, src), }; - // Initialize accumulator with signed zero for floating point parsing to - // work - let mut accum = if is_positive { _0 } else { -_1 }; - let mut last_accum = accum; // Necessary to detect overflow - let mut cs = src.chars().enumerate(); - let mut exp = None::<(char, uint)>; + // The significand to accumulate + let mut sig = if is_positive { _0 } else { -_1 }; + // Necessary to detect overflow + let mut prev_sig = sig; + let mut cs = src.chars().enumerate(); + // Exponent prefix and exponent index offset + let mut exp_info = None::<(char, uint)>; - // Parse integer part of number + // Parse the integer part of the significand for (i, c) in cs { - match c { - 'e' | 'E' | 'p' | 'P' => { - exp = Some((c, i + 1)); - break; // start of exponent - }, - '.' => { - break; // start of fractional part - }, - c => match c.to_digit(radix) { - Some(digit) => { - // shift accum one digit left - accum = accum * radix_gen; - - // add/subtract current digit depending on sign - if is_positive { - accum = accum + num::cast(digit as int).unwrap(); - } else { - accum = accum - num::cast(digit as int).unwrap(); - } + match c.to_digit(radix) { + Some(digit) => { + // shift significand one digit left + sig = sig * radix_t; + + // add/subtract current digit depending on sign + if is_positive { + sig = sig + num::cast(digit as int).unwrap(); + } else { + sig = sig - num::cast(digit as int).unwrap(); + } - // Detect overflow by comparing to last value, except - // if we've not seen any non-zero digits. - if last_accum != _0 { - if is_positive && accum <= last_accum { return Some(Float::infinity()); } - if !is_positive && accum >= last_accum { return Some(Float::neg_infinity()); } - - // Detect overflow by reversing the shift-and-add process - if is_positive && - (last_accum != ((accum - num::cast(digit as int).unwrap()) / radix_gen)) { - return Some(Float::infinity()); - } - if !is_positive && - (last_accum != ((accum + num::cast(digit as int).unwrap()) / radix_gen)) { - return Some(Float::neg_infinity()); - } - } - last_accum = accum; + // Detect overflow by comparing to last value, except + // if we've not seen any non-zero digits. + if prev_sig != _0 { + if is_positive && sig <= prev_sig + { return Some(Float::infinity()); } + if !is_positive && sig >= prev_sig + { return Some(Float::neg_infinity()); } + + // Detect overflow by reversing the shift-and-add process + let digit: T = num::cast(digit as int).unwrap(); + if is_positive && (prev_sig != ((sig - digit) / radix_t)) + { return Some(Float::infinity()); } + if !is_positive && (prev_sig != ((sig + digit) / radix_t)) + { return Some(Float::neg_infinity()); } + } + prev_sig = sig; + }, + None => match c { + 'e' | 'E' | 'p' | 'P' => { + exp_info = Some((c, i + 1)); + break; // start of exponent + }, + '.' => { + break; // start of fractional part }, - None => { - return None; // invalid number + _ => { + return None; }, }, } } - // Parse fractional part of number - // Skip if already reached start of exponent - if exp.is_none() { + // If we are not yet at the exponent parse the fractional + // part of the significand + if exp_info.is_none() { let mut power = _1; for (i, c) in cs { - match c { - 'e' | 'E' | 'p' | 'P' => { - exp = Some((c, i + 1)); - break; // start of exponent + match c.to_digit(radix) { + Some(digit) => { + let digit: T = num::cast(digit).unwrap(); + // Decrease power one order of magnitude + power = power / radix_t; + // add/subtract current digit depending on sign + sig = if is_positive { + sig + digit * power + } else { + sig - digit * power + }; + // Detect overflow by comparing to last value + if is_positive && sig < prev_sig + { return Some(Float::infinity()); } + if !is_positive && sig > prev_sig + { return Some(Float::neg_infinity()); } + prev_sig = sig; }, - c => match c.to_digit(radix) { - Some(digit) => { - let digit: T = num::cast(digit).unwrap(); - - // Decrease power one order of magnitude - power = power / radix_gen; - // add/subtract current digit depending on sign - accum = if is_positive { - accum + digit * power - } else { - accum - digit * power - }; - // Detect overflow by comparing to last value - if is_positive && accum < last_accum { return Some(Float::infinity()); } - if !is_positive && accum > last_accum { return Some(Float::neg_infinity()); } - last_accum = accum; + None => match c { + 'e' | 'E' | 'p' | 'P' => { + exp_info = Some((c, i + 1)); + break; // start of exponent }, - None => { + _ => { return None; // invalid number }, }, @@ -590,36 +540,41 @@ pub fn from_str_float( } } - let multiplier = match exp { - None => { - _1 // no exponent - }, + // Parse and calculate the exponent + let exp = match exp_info { Some((c, offset)) => { - let base: T = match (c, exponent) { - // c is never _ so don't need to handle specially - ('e', ExpDec) | ('E', ExpDec) => num::cast(10u).unwrap(), - ('p', ExpBin) | ('P', ExpBin) => num::cast(2u).unwrap(), - _ => return None, // char doesn't fit given exponent format + let base: T = match c { + 'E' | 'e' if radix == 10 => num::cast(10u).unwrap(), + 'P' | 'p' if radix == 16 => num::cast(2u).unwrap(), + _ => return None, }; - // parse remaining string as decimal integer - let exp = from_str::(src[offset..]); - match exp { - Some(exp_pow) if exp_pow < 0 => { - _1 / num::pow(base, (-exp_pow.to_int().unwrap()) as uint) - }, - Some(exp_pow) => { - num::pow(base, exp_pow.to_int().unwrap() as uint) - }, - None => { - return None; // invalid exponent - }, + + // Parse the exponent as decimal integer + let src = src[offset..]; + let (is_positive, exp) = match src.slice_shift_char() { + (Some('-'), src) => (false, from_str::(src)), + (Some('+'), src) => (true, from_str::(src)), + (Some(_), _) => (true, from_str::(src)), + (None, _) => return None, + }; + + match (is_positive, exp) { + (true, Some(exp)) => num::pow(base, exp), + (false, Some(exp)) => _1 / num::pow(base, exp), + (_, None) => return None, } }, + None => _1, // no exponent }; - Some(accum * multiplier) + + Some(sig * exp) } pub fn from_str_radix_int(src: &str, radix: uint) -> Option { + assert!(radix >= 2 && radix <= 36, + "from_str_radix_int: must lie in the range `[2, 36]` - found {}", + radix); + fn cast(x: uint) -> T { num::cast(x).unwrap() } @@ -687,10 +642,9 @@ mod test { assert_eq!(u, None); let s : Option = from_str_radix_int("80000", 10); assert_eq!(s, None); - let f : Option = from_str_float( - "10000000000000000000000000000000000000000", 10, false, ExpNone); + let f : Option = from_str_radix_float("10000000000000000000000000000000000000000", 10); assert_eq!(f, Some(Float::infinity())) - let fe : Option = from_str_float("1e40", 10, false, ExpDec); + let fe : Option = from_str_radix_float("1e40", 10); assert_eq!(fe, Some(Float::infinity())) } }