diff --git a/src/compiler.ts b/src/compiler.ts index af14b57934..02ffda461d 100644 --- a/src/compiler.ts +++ b/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); diff --git a/src/diagnosticMessages.json b/src/diagnosticMessages.json index 99486b1396..2e8efed630 100644 --- a/src/diagnosticMessages.json +++ b/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, diff --git a/src/module.ts b/src/module.ts index 07c061da5a..43530296bb 100644 --- a/src/module.ts +++ b/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); } diff --git a/tests/compiler/number.debug.wat b/tests/compiler/number.debug.wat index a91c2a331b..c2bf156863 100644 --- a/tests/compiler/number.debug.wat +++ b/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 diff --git a/tests/compiler/number.json b/tests/compiler/number.json index 1bdd02b1be..d823d836bf 100644 --- a/tests/compiler/number.json +++ b/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." ] } diff --git a/tests/compiler/number.ts b/tests/compiler/number.ts index cf7c76dbfa..84c45020c9 100644 --- a/tests/compiler/number.ts +++ b/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));