Repository: seaofvoices/darklua (rust)

Issue:
Preserve sign when evaluating zero
The `compute_expression` should not change the sign of `-0` to `0`, because it some cases it can change the sign of the result. 

Pull request:
Fix negative zero sign erasure
Closes #216 

This avoids the issue were the evaluator was replacing `-0` results with only `0`.

- [x] add entry to the changelog


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4b12b39..c265b49 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* fix negative zero sign erasure ([#222](https://github.com/seaofvoices/darklua/pull/222))
 * add `remove_if_expression` rule ([#221](https://github.com/seaofvoices/darklua/pull/221))
 
 ## 0.14.0
diff --git a/src/nodes/expressions/mod.rs b/src/nodes/expressions/mod.rs
index e66ef33..fc8bebf 100644
--- a/src/nodes/expressions/mod.rs
+++ b/src/nodes/expressions/mod.rs
@@ -101,7 +101,9 @@ impl From<f64> for Expression {
                 DecimalNumber::new(0.0),
             )
             .into(),
-            FpCategory::Zero => DecimalNumber::new(0.0).into(),
+            FpCategory::Zero => {
+                DecimalNumber::new(if value.is_sign_positive() { 0.0 } else { -0.0 }).into()
+            }
             FpCategory::Subnormal | FpCategory::Normal => {
                 if value < 0.0 {
                     UnaryExpression::new(UnaryOperator::Minus, Expression::from(value.abs())).into()
@@ -346,6 +348,7 @@ mod test {
             f64_1e42 => 1e42_f64,
             f64_infinity => f64::INFINITY,
             i64_minus_one => -1_i64,
+            f64_minus_zero => -0.0,
         );
     }
 }
diff --git a/src/process/evaluator/mod.rs b/src/process/evaluator/mod.rs
index 258fbf8..32ec733 100644
--- a/src/process/evaluator/mod.rs
+++ b/src/process/evaluator/mod.rs
@@ -477,6 +477,7 @@ mod test {
         false_expression(Expression::from(false)) => LuaValue::False,
         nil_expression(Expression::nil()) => LuaValue::Nil,
         number_expression(DecimalNumber::new(0.0)) => LuaValue::Number(0.0),
+        number_expression_negative_zero(DecimalNumber::new(-0.0)) => LuaValue::Number(-0.0),
         string_expression(StringExpression::from_value("foo")) => LuaValue::String("foo".to_owned()),
         empty_interpolated_string_expression(InterpolatedStringExpression::empty()) => LuaValue::String("".to_owned()),
         interpolated_string_expression_with_one_string(InterpolatedStringExpression::empty().with_segment("hello"))
@@ -550,6 +551,10 @@ mod test {
                                         (expect_float - result).abs() < f64::EPSILON,
                                         "{} does not approximate {}", result, expect_float
                                     );
+                                    assert!(
+                                        expect_float.is_sign_positive() == result.is_sign_positive(),
+                                        "{} should be of the same sign as {}", result, expect_float
+                                    );
                                 }
                             }
                             _ => {
@@ -667,6 +672,16 @@ mod test {
                 Expression::from(1.0),
                 Expression::from(0.0)
             ) => LuaValue::Number(f64::INFINITY),
+            negative_zero_plus_negative_zero(
+                BinaryOperator::Plus,
+                Expression::from(-0.0),
+                Expression::from(-0.0)
+            ) => LuaValue::Number(-0.0),
+            negative_zero_minus_zero(
+                BinaryOperator::Minus,
+                Expression::from(-0.0),
+                Expression::from(0.0)
+            ) => LuaValue::Number(-0.0),
             zero_divided_by_zero(
                 BinaryOperator::Slash,
                 Expression::from(0.0),
@@ -1038,6 +1053,7 @@ mod test {
             ) => LuaValue::False,
             not_identifier(Not, Expression::identifier("foo")) => LuaValue::Unknown,
             minus_one(Minus, DecimalNumber::new(1.0)) => LuaValue::from(-1.0),
+            minus_zero(Minus, DecimalNumber::new(-0.0)) => LuaValue::from(-0.0),
             minus_negative_number(Minus, DecimalNumber::new(-5.0)) => LuaValue::from(5.0),
             minus_string_converted_to_number(Minus, StringExpression::from_value("1")) => LuaValue::from(-1.0)
         );


Test patch:
diff --git a/src/nodes/expressions/snapshots/darklua_core__nodes__expressions__test__expression_from_floats__f64_minus_zero.snap b/src/nodes/expressions/snapshots/darklua_core__nodes__expressions__test__expression_from_floats__f64_minus_zero.snap
new file mode 100644
index 0000000..527deb5
--- /dev/null
+++ b/src/nodes/expressions/snapshots/darklua_core__nodes__expressions__test__expression_from_floats__f64_minus_zero.snap
@@ -0,0 +1,13 @@
+---
+source: src/nodes/expressions/mod.rs
+expression: result
+---
+Number(
+    Decimal(
+        DecimalNumber {
+            float: -0.0,
+            exponent: None,
+            token: None,
+        },
+    ),
+)
diff --git a/tests/rule_tests/compute_expression.rs b/tests/rule_tests/compute_expression.rs
index 8609827..dd115bd 100644
--- a/tests/rule_tests/compute_expression.rs
+++ b/tests/rule_tests/compute_expression.rs
@@ -34,11 +34,15 @@ test_rule!(
         => "return 'is equal'",
     if_expression_elseif_always_false("return if false then 'is true' elseif 1 == 2 then 'is equal' else nil")
         => "return nil",
+    preserve_negative_zero("return -0") => "return -0",
+    addition_preserve_negative_zero("return -0 + -0") => "return -0",
+    subtract_preserve_negative_zero("return -0 - 0") => "return -0",
 );
 
-test_rule_without_effects!(if_expression_unknown_condition(
-    "return if condition then func() else func2()"
-),);
+test_rule_without_effects!(
+    ComputeExpression::default(),
+    if_expression_unknown_condition("return if condition then func() else func2()"),
+);
 
 #[test]
 fn deserialize_from_object_notation() {