Cleanup.

Raw f64 in some more places, use rust std f64 parsing for numbers.

Author: kaj

Diff for: rsass/src/parser/value.rs

@@ -10,18 +10,18 @@ use super::{
     input_to_string, list_or_single, position, sass_string, PResult, Span,
 };
 use crate::sass::{BinOp, SassString, Value};
-use crate::value::{ListSeparator, Number, Numeric, Operator, Rgba};
+use crate::value::{ListSeparator, Numeric, Operator, Rgba};
 use nom::branch::alt;
 use nom::bytes::complete::{tag, tag_no_case};
 use nom::character::complete::{
     alphanumeric1, char, digit1, multispace0, multispace1, one_of,
 };
 use nom::combinator::{
-    cut, into, map, map_res, not, opt, peek, recognize, success, value,
+    cut, into, map, map_opt, map_res, not, opt, peek, recognize, value,
     verify,
 };
 use nom::error::context;
-use nom::multi::{fold_many0, fold_many1, many0, many_m_n, separated_list1};
+use nom::multi::{fold_many0, many0, many_m_n, separated_list1};
 use nom::sequence::{delimited, pair, preceded, terminated, tuple};
 use std::str::from_utf8;
 
@@ -332,68 +332,20 @@ pub fn numeric(input: Span) -> PResult<Numeric> {
     })(input)
 }
 
-pub fn number(input: Span) -> PResult<Number> {
-    map(
-        tuple((
-            sign_neg,
+pub fn number(input: Span) -> PResult<f64> {
+    map_opt(
+        recognize(delimited(
+            opt(one_of("+-")),
             alt((
-                map(pair(decimal_integer, decimal_decimals), |(n, d)| n + d),
-                decimal_decimals,
-                decimal_integer,
-            )),
-            opt(preceded(
-                alt((tag("e"), tag("E"))),
-                tuple((sign_neg, decimal_i32)),
+                terminated(digit1, opt(terminated(char('.'), digit1))),
+                preceded(char('.'), digit1),
             )),
+            opt(delimited(one_of("eE"), opt(one_of("+-")), digit1)),
         )),
-        |(is_neg, num, exp)| {
-            let value = if is_neg { -num } else { num };
-            Number::from(if let Some((e_neg, e_val)) = exp {
-                let e_val = if e_neg { -e_val } else { e_val };
-                // Note: powi sounds right, but looses some precision.
-                value * 10f64.powf(e_val.into())
-            } else {
-                value
-            })
-        },
-    )(input)
-}
-
-/// Parse true on `-` and false on `+` or no sign.
-fn sign_neg(input: Span) -> PResult<bool> {
-    alt((
-        value(true, char('-')),
-        value(false, char('+')),
-        success(false),
-    ))(input)
-}
-
-pub fn decimal_integer(input: Span) -> PResult<f64> {
-    map(digit1, |s: Span| {
-        s.fragment()
-            .iter()
-            .fold(0.0, |r, d| (r * 10.) + f64::from(d - b'0'))
-    })(input)
-}
-pub fn decimal_i32(input: Span) -> PResult<i32> {
-    fold_many1(
-        // Note: We should use bytes directly, one_of returns a char.
-        one_of("0123456789"),
-        || 0,
-        |r, d| (r * 10) + i32::from(d as u8 - b'0'),
+        |s: Span| from_utf8(s.fragment()).ok()?.parse().ok(),
     )(input)
 }
 
-pub fn decimal_decimals(input: Span) -> PResult<f64> {
-    map(preceded(char('.'), digit1), |s: Span| {
-        let digits = s.fragment();
-        digits
-            .iter()
-            .fold(0.0, |r, d| (r * 10.) + f64::from(d - b'0'))
-            * (10f64).powf(-(digits.len() as f64))
-    })(input)
-}
-
 pub fn variable_nomod(input: Span) -> PResult<Value> {
     let (rest, name) = preceded(char('$'), identifier)(input)?;
     let pos = input.up_to(&rest).to_owned();

Diff for: rsass/src/value/numeric.rs

@@ -55,7 +55,7 @@ impl Numeric {
     pub fn as_unit(&self, unit: Unit) -> Option<Number> {
         self.unit
             .scale_to_unit(&unit)
-            .map(|scale| &self.value * &scale)
+            .map(|scale| &self.value * &Number::from(scale))
     }
     /// Convert this numeric value to a given unit, if possible.
     ///
@@ -67,7 +67,9 @@ impl Numeric {
     /// assert_eq!(inch.as_unit(Unit::Deg), None);
     /// ```
     pub fn as_unitset(&self, unit: &UnitSet) -> Option<Number> {
-        self.unit.scale_to(unit).map(|scale| &self.value * &scale)
+        self.unit
+            .scale_to(unit)
+            .map(|scale| &self.value * &Number::from(scale))
     }
 
     /// Convert this numeric value to a given unit, if possible.

Diff for: rsass/src/value/unit.rs

@@ -1,6 +1,5 @@
 //! The Unit enum defines css units
 
-use crate::value::Number;
 use std::f64::consts::FRAC_1_PI;
 use std::fmt;
 
@@ -150,9 +149,9 @@ impl Unit {
     /// Get a scaling factor to convert this unit to another unit.
     ///
     /// Returns None if the units are of different dimension.
-    pub fn scale_to(&self, other: &Self) -> Option<Number> {
+    pub fn scale_to(&self, other: &Self) -> Option<f64> {
         if self == other {
-            Some(1.into())
+            Some(1.)
         } else if self.dimension() == other.dimension() {
             Some(self.scale_factor() / other.scale_factor())
         } else {
@@ -163,9 +162,9 @@ impl Unit {
     /// Some of these are exact and correct, others are more arbitrary.
     /// When comparing 10cm to 4in, these factors will give correct results.
     /// When comparing rems to vw, who can say?
-    pub(crate) fn scale_factor(&self) -> Number {
+    pub(crate) fn scale_factor(&self) -> f64 {
         #[allow(clippy::match_same_arms)]
-        Number::from(match *self {
+        match *self {
             Self::Em | Self::Rem => 10. / 2.,
             Self::Ex => 10. / 3.,
             Self::Ch => 10. / 4.,
@@ -198,7 +197,7 @@ impl Unit {
             Self::None => 1.,
 
             Self::Unknown(_) => 1.,
-        })
+        }
     }
 }
 

Diff for: rsass/src/value/unitset.rs

@@ -83,15 +83,15 @@ impl UnitSet {
     /// Get a scaling factor to convert this unit to another unit.
     ///
     /// Returns None if the units are of different dimension.
-    pub fn scale_to(&self, other: &Self) -> Option<Number> {
+    pub fn scale_to(&self, other: &Self) -> Option<f64> {
         if let [(u, 1)] = other.units.as_slice() {
             self.scale_to_unit(u)
         } else if other.is_none() {
             self.scale_to_unit(&Unit::None)
         } else {
             let quote = self / other;
             if quote.dimension().is_empty() {
-                Some(quote.units.iter().fold(1.into(), |a, (unit, power)| {
+                Some(quote.units.iter().fold(1., |a, (unit, power)| {
                     a * unit.scale_factor().powi((*power).into())
                 }))
             } else {
@@ -103,7 +103,7 @@ impl UnitSet {
     /// Get a scaling factor to convert this unit to another unit.
     ///
     /// Returns None if the units are of different dimension.
-    pub fn scale_to_unit(&self, other: &Unit) -> Option<Number> {
+    pub fn scale_to_unit(&self, other: &Unit) -> Option<f64> {
         if let [(u, 1)] = self.units.as_slice() {
             u.scale_to(other)
         } else if self.is_none() {
@@ -116,19 +116,19 @@ impl UnitSet {
 
     /// Simplify this unit set, returning a scaling factor.
     pub fn simplify(&mut self) -> Number {
-        let mut factor = 1.into();
+        let mut factor = 1.;
         if self.units.len() > 1 {
             for i in 1..(self.units.len()) {
                 let (a, b) = self.units.split_at_mut(i);
                 let (au, ap) = a.last_mut().unwrap();
                 for (bu, bp) in b {
                     if let Some(f) = bu.scale_to(au) {
                         if ap.abs() > bp.abs() {
-                            factor = factor * f.powi((*bp).into());
+                            factor *= f.powi((*bp).into());
                             *ap += *bp;
                             *bp = 0;
                         } else {
-                            factor = factor / f.powi((*ap).into());
+                            factor /= f.powi((*ap).into());
                             *bp += *ap;
                             *ap = 0;
                         }
@@ -137,7 +137,7 @@ impl UnitSet {
             }
         }
         self.units.retain(|(_u, p)| *p != 0);
-        factor
+        factor.into()
     }
 }
 

Diff for: rsass/tests/misc/fuzz_cases.rs

@@ -15,7 +15,7 @@ fn decimal_integer_overflow() {
     assert_eq!(
         compile_value(b"2000000000000000000000000000000000000", FORMAT)
             .unwrap(),
-        b"1999999999999999800000000000000000000".to_vec(),
+        b"2000000000000000000000000000000000000".to_vec(),
     );
 }
 

Diff for: rsass/tests/spec/values/numbers/bounds.rs

@@ -14,7 +14,6 @@ mod int {
         use super::runner;
 
         #[test]
-        #[ignore] // wrong result
         fn slightly() {
             assert_eq!(
         runner().ok(
@@ -44,7 +43,6 @@ mod int {
         use super::runner;
 
         #[test]
-        #[ignore] // wrong result
         fn slightly() {
             assert_eq!(
         runner().ok(
@@ -70,7 +68,6 @@ mod int {
         }
     }
     #[test]
-    #[ignore] // wrong result
     fn max_value() {
         assert_eq!(
         runner().ok(
@@ -82,7 +79,6 @@ mod int {
     );
     }
     #[test]
-    #[ignore] // wrong result
     fn min_value() {
         assert_eq!(
         runner().ok(
@@ -194,7 +190,6 @@ mod precision_limit {
         use super::runner;
 
         #[test]
-        #[ignore] // wrong result
         fn after_decimal() {
             assert_eq!(
         runner().ok(