feat: Add warning about NaN invariant and -0.0 for direct comparisons

src/compiler.ts

@@ -52,7 +52,9 @@ import {
   SideEffects,
   SwitchBuilder,
   ExpressionRunnerFlags,
-  isConstZero
+  isConstZero,
+  isConstNegZero,
+  isConstExpressionNaN
 } from "./module";
 
 import {
@@ -3992,7 +3994,23 @@ export class Compiler extends DiagnosticEmitter {
           this.currentType = contextualType;
           return module.unreachable();
         }
-
+        if (commonType.isFloatValue) {
+          if (
+            isConstExpressionNaN(module, rightExpr) ||
+            isConstExpressionNaN(module, leftExpr)
+          ) {
+            this.warning(
+              DiagnosticCode._NaN_does_not_compare_equal_to_any_other_value_including_itself_Use_isNaN_x_instead,
+              expression.range
+            );
+          }
+          if (isConstNegZero(rightExpr) || isConstNegZero(leftExpr)) {
+            this.warning(
+              DiagnosticCode.Comparison_with_0_0_is_sign_insensitive_Use_Object_is_x_0_0_if_the_sign_matters,
+              expression.range
+            );
+          }
+        }
         leftExpr = this.convertExpression(leftExpr, leftType, commonType, false, left);
         leftType = commonType;
         rightExpr = this.convertExpression(rightExpr, rightType, commonType, false, right);
@@ -4028,7 +4046,23 @@ export class Compiler extends DiagnosticEmitter {
           this.currentType = contextualType;
           return module.unreachable();
         }
-
+        if (commonType.isFloatValue) {
+          if (
+            isConstExpressionNaN(module, rightExpr) ||
+            isConstExpressionNaN(module, leftExpr)
+          ) {
+            this.warning(
+              DiagnosticCode._NaN_does_not_compare_equal_to_any_other_value_including_itself_Use_isNaN_x_instead,
+              expression.range
+            );
+          }
+          if (isConstNegZero(rightExpr) || isConstNegZero(leftExpr)) {
+            this.warning(
+              DiagnosticCode.Comparison_with_0_0_is_sign_insensitive_Use_Object_is_x_0_0_if_the_sign_matters,
+              expression.range
+            );
+          }
+        }
         leftExpr = this.convertExpression(leftExpr, leftType, commonType, false, left);
         leftType = commonType;
         rightExpr = this.convertExpression(rightExpr, rightType, commonType, false, right);

src/diagnosticMessages.json

@@ -56,6 +56,8 @@
   "Indexed access may involve bounds checking.": 904,
   "Explicitly returning constructor drops 'this' allocation.": 905,
   "Unnecessary definite assignment.": 906,
+  "'NaN' does not compare equal to any other value including itself. Use isNaN(x) instead.": 907,
+  "Comparison with -0.0 is sign insensitive. Use Object.is(x, -0.0) if the sign matters.": 908,
 
   "Unterminated string literal.": 1002,
   "Identifier expected.": 1003,

src/module.ts

@@ -2788,7 +2788,7 @@ export class Module {
     maxLoopIterations: i32 = 1
   ): ExpressionRef {
     var runner = binaryen._ExpressionRunnerCreate(this.ref, flags, maxDepth, maxLoopIterations);
-    var precomp =  binaryen._ExpressionRunnerRunAndDispose(runner, expr);
+    var precomp = binaryen._ExpressionRunnerRunAndDispose(runner, expr);
     if (precomp) {
       if (!this.isConstExpression(precomp)) return 0;
       assert(getExpressionType(precomp) == getExpressionType(expr));
@@ -2810,7 +2810,11 @@ export class Module {
             case BinaryOp.MulI32:
             case BinaryOp.AddI64:
             case BinaryOp.SubI64:
-            case BinaryOp.MulI64: return this.isConstExpression(getBinaryLeft(expr)) && this.isConstExpression(getBinaryRight(expr));
+            case BinaryOp.MulI64:
+              return (
+                this.isConstExpression(getBinaryLeft(expr)) &&
+                this.isConstExpression(getBinaryRight(expr))
+              );
           }
         }
         break;
@@ -2934,6 +2938,52 @@ export function isConstNonZero(expr: ExpressionRef): bool {
   return false;
 }
 
+export function isConstNegZero(expr: ExpressionRef): bool {
+  if (getExpressionId(expr) != ExpressionId.Const) return false;
+  var type = getExpressionType(expr);
+  if (type == TypeRef.F32) {
+    let d = getConstValueF32(expr);
+    return d == 0 && f32_as_i32(d) < 0;
+  }
+  if (type == TypeRef.F64) {
+    let d = getConstValueF64(expr);
+    return d == 0 && i64_signbit(f64_as_i64(d));
+  }
+  return false;
+}
+
+export function isConstNaN(expr: ExpressionRef): bool {
+  if (getExpressionId(expr) != ExpressionId.Const) return false;
+  var type = getExpressionType(expr);
+  if (type == TypeRef.F32) return isNaN(getConstValueF32(expr));
+  if (type == TypeRef.F64) return isNaN(getConstValueF64(expr));
+  return false;
+}
+
+export function isConstExpressionNaN(module: Module, expr: ExpressionRef): bool {
+  var id = getExpressionId(expr);
+  var type = getExpressionType(expr);
+  if (type == TypeRef.F32 || type == TypeRef.F64) {
+    if (id == ExpressionId.Const) {
+      return isNaN(
+        type == TypeRef.F32
+          ? getConstValueF32(expr)
+          : getConstValueF64(expr)
+      );
+    } else if (id == ExpressionId.GlobalGet) {
+      let precomp = module.runExpression(expr, ExpressionRunnerFlags.Default, 8);
+      if (precomp) {
+        return isNaN(
+          type == TypeRef.F32
+            ? getConstValueF32(precomp)
+            : getConstValueF64(precomp)
+        );
+      }
+    }
+  }
+  return false;
+}
+
 export function getLocalGetIndex(expr: ExpressionRef): Index {
   return binaryen._BinaryenLocalGetGetIndex(expr);
 }

tests/compiler/number.debug.wat

@@ -4972,6 +4972,35 @@
    call $~lib/builtins/abort
    unreachable
   end
+  f64.const 1
+  global.get $~lib/builtins/f32.NaN
+  f64.promote_f32
+  f64.eq
+  i32.eqz
+  drop
+  global.get $~lib/number/F32.NaN
+  f32.const nan:0x400000
+  f32.eq
+  i32.eqz
+  drop
+  f64.const nan:0x8000000000000
+  f64.const 1
+  f64.eq
+  i32.eqz
+  drop
+  f64.const 1
+  global.get $~lib/builtins/f32.NaN
+  f64.promote_f32
+  f64.ne
+  drop
+  global.get $~lib/number/F32.NaN
+  f32.const nan:0x400000
+  f32.ne
+  drop
+  f64.const nan:0x8000000000000
+  f64.const 1
+  f64.ne
+  drop
   global.get $~lib/memory/__stack_pointer
   i32.const 8
   i32.add

tests/compiler/number.json

@@ -1,4 +1,18 @@
 {
   "asc_flags": [
+  ],
+  "stderr": [
+    "AS907: 'NaN' does not compare equal to any other value including itself. Use isNaN(x) instead.",
+    "AS907: 'NaN' does not compare equal to any other value including itself. Use isNaN(x) instead.",
+    "AS907: 'NaN' does not compare equal to any other value including itself. Use isNaN(x) instead.",
+    "AS907: 'NaN' does not compare equal to any other value including itself. Use isNaN(x) instead.",
+    "AS907: 'NaN' does not compare equal to any other value including itself. Use isNaN(x) instead.",
+    "AS907: 'NaN' does not compare equal to any other value including itself. Use isNaN(x) instead.",
+    "AS908: Comparison with -0.0 is sign insensitive. Use Object.is(x, -0.0) if the sign matters.",
+    "AS908: Comparison with -0.0 is sign insensitive. Use Object.is(x, -0.0) if the sign matters.",
+    "AS908: Comparison with -0.0 is sign insensitive. Use Object.is(x, -0.0) if the sign matters.",
+    "AS908: Comparison with -0.0 is sign insensitive. Use Object.is(x, -0.0) if the sign matters.",
+    "AS908: Comparison with -0.0 is sign insensitive. Use Object.is(x, -0.0) if the sign matters.",
+    "AS908: Comparison with -0.0 is sign insensitive. Use Object.is(x, -0.0) if the sign matters."
   ]
 }

tests/compiler/number.ts

@@ -65,3 +65,23 @@ assert(F64.isInteger(f64.MIN_SAFE_INTEGER) == true);
 assert(F64.isInteger(f64.MAX_SAFE_INTEGER) == true);
 assert(F64.isInteger(+0.5) == false);
 assert(F64.isInteger(-1.5) == false);
+
+// Should produce warnings
+
+// always false
+assert(!(1.0 == NaN));
+assert(!(NaN == F32.NaN));
+assert(!(F64.NaN == 1.0));
+
+// always true
+assert(1.0 != NaN);
+assert(NaN != F32.NaN);
+assert(f64.NaN != 1.0);
+
+// always true
+assert(+.0 == -.0);
+assert(-.0 != -.0);
+assert(-.0 == +.0);
+assert(f32(+.0) == f32(-.0));
+assert(f32(-.0) != f32(-.0));
+assert(f32(-.0) == f32(+.0));