Bug 1924363 - Eagerly resolve sum nodes during parsing r=layout-reviewers,emilio

Product nodes are eagerly resolved during parse time, but sum nodes are
not. This might cause floating point inprecision in sum nodes, which
leads to invalid calculations, e.g. `round(down, (7 - 1) / 3, 1)` would
end up being `round(down, (2.3333333 - 0.33333334), 1)`, then
`round(down, 1.99999996, 1)`, which equals `1`, which is incorrect.

Differential Revision: https://phabricator.services.mozilla.com/D225936
This commit is contained in:
Tiaan Louw 2024-10-17 10:09:01 +00:00
parent ebdc900582
commit 23fd208d8f
3 changed files with 14 additions and 4 deletions

View File

@ -665,7 +665,7 @@ impl<L: CalcNodeLeaf> CalcNode<L> {
}
/// Tries to merge one node into another using the sum, that is, perform `x` + `y`.
fn try_sum_in_place(&mut self, other: &Self) -> Result<(), ()> {
pub fn try_sum_in_place(&mut self, other: &Self) -> Result<(), ()> {
match (self, other) {
(&mut CalcNode::Leaf(ref mut one), &CalcNode::Leaf(ref other)) => {
one.try_sum_in_place(other)

View File

@ -768,12 +768,17 @@ impl CalcNode {
}
match *input.next()? {
Token::Delim('+') => {
sum.push(Self::parse_product(context, input, allowed_units)?);
let rhs = Self::parse_product(context, input, allowed_units)?;
if sum.last_mut().unwrap().try_sum_in_place(&rhs).is_err() {
sum.push(rhs);
}
},
Token::Delim('-') => {
let mut rhs = Self::parse_product(context, input, allowed_units)?;
rhs.negate();
sum.push(rhs);
if sum.last_mut().unwrap().try_sum_in_place(&rhs).is_err() {
sum.push(rhs);
}
},
_ => {
input.reset(&start);

View File

@ -246,4 +246,9 @@ test_minus_infinity("round(down, -1, infinity)");
test_minus_zero("round(down, -1 * 0, infinity)");
test_plus_zero("round(down, 0, infinity)");
test_plus_zero("round(down, 1, infinity)");
</script>
// In this test if the multiplication is factored into the sum first, the
// result of the round will be incorrect, because of floating point inprecision.
// round(down, 2.3333333 - 0.33333334, 1) = round(down, 1.99999996, 1) = 1
test_math_used('round(down, (7 - 1) / 3, 1)', '2', {type:'number'});
</script>