diff options
author | Kyle Farnung <kfarnung@microsoft.com> | 2018-03-15 17:22:30 -0700 |
---|---|---|
committer | Myles Borins <mylesborins@google.com> | 2018-04-16 17:46:19 -0400 |
commit | 3aa5b7d939409bb5abab0fd4478e44f4e643e297 (patch) | |
tree | 932eaf7104c63c8cfa920fbb0dfe4b1ef751cc33 | |
parent | abd9fd67976e1da0ffc7deb90832a5d5b7511fe0 (diff) | |
download | node-new-3aa5b7d939409bb5abab0fd4478e44f4e643e297.tar.gz |
n-api: add more `int64_t` tests
* Updated tests for `Number` and `int32_t`
* Added new tests for `int64_t`
* Updated N-API `int64_t` behavior to return zero for all non-finite
numbers
* Clarified the documentation for these calls.
Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/19402
Refs: https://github.com/nodejs/node-chakracore/pull/500
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
-rw-r--r-- | doc/api/n-api.md | 27 | ||||
-rw-r--r-- | src/node_api.cc | 23 | ||||
-rw-r--r-- | test/addons-napi/test_number/test.js | 135 |
3 files changed, 135 insertions, 50 deletions
diff --git a/doc/api/n-api.md b/doc/api/n-api.md index e176cbdf1e..38d7b22f55 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -1806,13 +1806,17 @@ napi_status napi_get_value_int32(napi_env env, - `[out] result`: C int32 primitive equivalent of the given JavaScript Number. Returns `napi_ok` if the API succeeded. If a non-number `napi_value` -is passed in `napi_number_expected . +is passed in `napi_number_expected`. This API returns the C int32 primitive equivalent -of the given JavaScript Number. If the number exceeds the range of the -32 bit integer, then the result is truncated to the equivalent of the -bottom 32 bits. This can result in a large positive number becoming -a negative number if the value is > 2^31 -1. +of the given JavaScript Number. + +If the number exceeds the range of the 32 bit integer, then the result is +truncated to the equivalent of the bottom 32 bits. This can result in a large +positive number becoming a negative number if the value is > 2^31 -1. + +Non-finite number values (NaN, positive infinity, or negative infinity) set the +result to zero. #### napi_get_value_int64 <!-- YAML @@ -1831,8 +1835,17 @@ napi_status napi_get_value_int64(napi_env env, Returns `napi_ok` if the API succeeded. If a non-number `napi_value` is passed in it returns `napi_number_expected`. -This API returns the C int64 primitive equivalent of the given -JavaScript Number. +This API returns the C int64 primitive equivalent of the given JavaScript +Number. + +Number values outside the range of +[`Number.MIN_SAFE_INTEGER`](https://tc39.github.io/ecma262/#sec-number.min_safe_integer) +-(2^53 - 1) - +[`Number.MAX_SAFE_INTEGER`](https://tc39.github.io/ecma262/#sec-number.max_safe_integer) +(2^53 - 1) will lose precision. + +Non-finite number values (NaN, positive infinity, or negative infinity) set the +result to zero. #### napi_get_value_string_latin1 <!-- YAML diff --git a/src/node_api.cc b/src/node_api.cc index a13398006b..fbe468b5a3 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -2193,15 +2193,24 @@ napi_status napi_get_value_int64(napi_env env, RETURN_STATUS_IF_FALSE(env, val->IsNumber(), napi_number_expected); - // v8::Value::IntegerValue() converts NaN to INT64_MIN, inconsistent with - // v8::Value::Int32Value() that converts NaN to 0. So special-case NaN here. + // v8::Value::IntegerValue() converts NaN, +Inf, and -Inf to INT64_MIN, + // inconsistent with v8::Value::Int32Value() which converts those values to 0. + // Special-case all non-finite values to match that behavior. double doubleValue = val.As<v8::Number>()->Value(); - if (std::isnan(doubleValue)) { - *result = 0; + if (std::isfinite(doubleValue)) { + // v8::Value::IntegerValue() as shipped with v6.x returns inconsistent + // values outside of the int64_t range. We rectify that here. + if (doubleValue >= static_cast<double>(INT64_MAX)) { + *result = INT64_MAX; + } else if (doubleValue <= static_cast<double>((int64_t)-INT64_MAX - 1)) { + *result = -INT64_MAX - 1; + } else { + // Empty context: https://github.com/nodejs/node/issues/14379 + v8::Local<v8::Context> context; + *result = val->IntegerValue(context).FromJust(); + } } else { - // Empty context: https://github.com/nodejs/node/issues/14379 - v8::Local<v8::Context> context; - *result = val->IntegerValue(context).FromJust(); + *result = 0; } return napi_clear_last_error(env); diff --git a/test/addons-napi/test_number/test.js b/test/addons-napi/test_number/test.js index f9d6eb6a6f..3dbbb64932 100644 --- a/test/addons-napi/test_number/test.js +++ b/test/addons-napi/test_number/test.js @@ -5,50 +5,113 @@ const test_number = require(`./build/${common.buildType}/test_number`); // testing api calls for number -assert.strictEqual(0, test_number.Test(0)); -assert.strictEqual(1, test_number.Test(1)); -assert.strictEqual(-1, test_number.Test(-1)); -assert.strictEqual(100, test_number.Test(100)); -assert.strictEqual(2121, test_number.Test(2121)); -assert.strictEqual(-1233, test_number.Test(-1233)); -assert.strictEqual(986583, test_number.Test(986583)); -assert.strictEqual(-976675, test_number.Test(-976675)); +function testNumber(num) { + assert.strictEqual(num, test_number.Test(num)); +} -const num1 = 98765432213456789876546896323445679887645323232436587988766545658; -assert.strictEqual(num1, test_number.Test(num1)); +testNumber(0); +testNumber(-0); +testNumber(1); +testNumber(-1); +testNumber(100); +testNumber(2121); +testNumber(-1233); +testNumber(986583); +testNumber(-976675); -const num2 = -4350987086545760976737453646576078997096876957864353245245769809; -assert.strictEqual(num2, test_number.Test(num2)); +testNumber( + 98765432213456789876546896323445679887645323232436587988766545658); +testNumber( + -4350987086545760976737453646576078997096876957864353245245769809); +testNumber(Number.MIN_SAFE_INTEGER); +testNumber(Number.MAX_SAFE_INTEGER); +testNumber(Number.MAX_SAFE_INTEGER + 10); -const num3 = Number.MAX_SAFE_INTEGER; -assert.strictEqual(num3, test_number.Test(num3)); +testNumber(Number.MIN_VALUE); +testNumber(Number.MAX_VALUE); +testNumber(Number.MAX_VALUE + 10); -const num4 = Number.MAX_SAFE_INTEGER + 10; -assert.strictEqual(num4, test_number.Test(num4)); +testNumber(Number.POSITIVE_INFINITY); +testNumber(Number.NEGATIVE_INFINITY); +assert(Object.is(NaN, test_number.Test(NaN))); -const num5 = Number.MAX_VALUE; -assert.strictEqual(num5, test_number.Test(num5)); +// validate documented behavior when value is retrieved as 32-bit integer with +// `napi_get_value_int32` +function testInt32(input, expected = input) { + assert.strictEqual(expected, test_number.TestInt32Truncation(input)); +} -const num6 = Number.MAX_VALUE + 10; -assert.strictEqual(num6, test_number.Test(num6)); +// Test zero +testInt32(0.0, 0); +testInt32(-0.0, 0); -const num7 = Number.POSITIVE_INFINITY; -assert.strictEqual(num7, test_number.Test(num7)); +// Test min/max int32 range +testInt32(-Math.pow(2, 31)); +testInt32(Math.pow(2, 31) - 1); -const num8 = Number.NEGATIVE_INFINITY; -assert.strictEqual(num8, test_number.Test(num8)); +// Test overflow scenarios +testInt32(4294967297, 1); +testInt32(4294967296, 0); +testInt32(4294967295, -1); +testInt32(4294967296 * 5 + 3, 3); +// Test min/max safe integer range +testInt32(Number.MIN_SAFE_INTEGER, 1); +testInt32(Number.MAX_SAFE_INTEGER, -1); -// validate documented behaviour when value is retrieved -// as 32 bit integer with napi_get_value_int32 -assert.strictEqual(1, test_number.TestInt32Truncation(4294967297)); -assert.strictEqual(0, test_number.TestInt32Truncation(4294967296)); -assert.strictEqual(-1, test_number.TestInt32Truncation(4294967295)); -assert.strictEqual(3, test_number.TestInt32Truncation(4294967296 * 5 + 3)); +// Test within int64_t range (with precision loss) +testInt32(-Math.pow(2, 63) + (Math.pow(2, 9) + 1), 1024); +testInt32(Math.pow(2, 63) - (Math.pow(2, 9) + 1), -1024); -// validate that the boundaries of safe integer can be passed through -// successfully -assert.strictEqual(Number.MAX_SAFE_INTEGER, - test_number.TestInt64Truncation(Number.MAX_SAFE_INTEGER)); -assert.strictEqual(Number.MIN_SAFE_INTEGER, - test_number.TestInt64Truncation(Number.MIN_SAFE_INTEGER)); +// Test min/max double value +testInt32(-Number.MIN_VALUE, 0); +testInt32(Number.MIN_VALUE, 0); +testInt32(-Number.MAX_VALUE, 0); +testInt32(Number.MAX_VALUE, 0); + +// Test outside int64_t range +testInt32(-Math.pow(2, 63) + (Math.pow(2, 9)), 0); +testInt32(Math.pow(2, 63) - (Math.pow(2, 9)), 0); + +// Test non-finite numbers +testInt32(Number.POSITIVE_INFINITY, 0); +testInt32(Number.NEGATIVE_INFINITY, 0); +testInt32(Number.NaN, 0); + +// validate documented behavior when value is retrieved as 64-bit integer with +// `napi_get_value_int64` +function testInt64(input, expected = input) { + assert.strictEqual(expected, test_number.TestInt64Truncation(input)); +} + +// Both V8 and ChakraCore return a sentinel value of `0x8000000000000000` when +// the conversion goes out of range, but V8 treats it as unsigned in some cases. +const RANGEERROR_POSITIVE = Math.pow(2, 63); +const RANGEERROR_NEGATIVE = -Math.pow(2, 63); + +// Test zero +testInt64(0.0, 0); +testInt64(-0.0, 0); + +// Test min/max safe integer range +testInt64(Number.MIN_SAFE_INTEGER); +testInt64(Number.MAX_SAFE_INTEGER); + +// Test within int64_t range (with precision loss) +testInt64(-Math.pow(2, 63) + (Math.pow(2, 9) + 1)); +testInt64(Math.pow(2, 63) - (Math.pow(2, 9) + 1)); + +// Test min/max double value +testInt64(-Number.MIN_VALUE, 0); +testInt64(Number.MIN_VALUE, 0); +testInt64(-Number.MAX_VALUE, RANGEERROR_NEGATIVE); +testInt64(Number.MAX_VALUE, RANGEERROR_POSITIVE); + +// Test outside int64_t range +testInt64(-Math.pow(2, 63) + (Math.pow(2, 9)), RANGEERROR_NEGATIVE); +testInt64(Math.pow(2, 63) - (Math.pow(2, 9)), RANGEERROR_POSITIVE); + +// Test non-finite numbers +testInt64(Number.POSITIVE_INFINITY, 0); +testInt64(Number.NEGATIVE_INFINITY, 0); +testInt64(Number.NaN, 0); |