Normative: Permit BigInt(large Number) #138
Conversation
spec.html
Outdated
| <emu-alg> | ||
| 1. Assert: Type(_number_) is Number. | ||
| 1. If _number_ is *NaN*, *+∞*, or *-∞*, return *false*. | ||
| 1. Let _integer_ be ! ToInteger(_number_). | ||
| 1. If ! SameValueZero(_integer_, _number_) is *false*, return *false*. | ||
| 1. If abs(_integer_) ≤ 2<sup>53</sup>-1, return *true*. | ||
| 1. Otherwise, return *false*. |
There was a problem hiding this comment.
This algorithm will never return true. I think you want this last step to return true.
There was a problem hiding this comment.
Oops! Thanks for catching this. That's what I get for pushing a PR late at night.
Previously, the BigInt constructor used IsSafeInteger to check if a Number is safe to cast to a BigInt without losing information. This check is unnecessarily conservative--it rules out large integer-valued Numbers. This patch switches to just check that the Number is an integer value. This patch specifies the resolution of the discussion in the March 2018 TC39 meeting. Closes #132
|
@littledan just to double check, this doesn't change the current behavior/tests in test 262, right? |
|
@caiolima This is a normative change. I don't know whether there is a test262 test that hits this case. If there isn't any test to update, it'd be good to add a test for it. |
|
@caiolima Would you be interested in writing a test262 test for this change? |
|
@jakobkummerow has implemented this change in V8 at https://chromium-review.googlesource.com/c/v8/v8/+/1004120 . Now that it's been proven out to this degree, and we also have committee consensus, I'm landing this patch; I'm sure we'll catch up soon on test262 tests. |
Spec change: tc39/proposal-bigint#138 Bug: v8:6791 Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng Change-Id: I7367273ed1e98971be3b277f6486333a96412185 Reviewed-on: https://chromium-review.googlesource.com/1004120 Commit-Queue: Jakob Kummerow <[email protected]> Reviewed-by: Georg Neis <[email protected]> Cr-Commit-Position: refs/heads/master@{#52611}
- "CannotSuspendMainAgent" feature was changed to "CanBlockIsFalse" flag - Move annex-b tests into annex-b directory - Update variable names in nonshared-int-views.js tests - Move getReport() call in nan-for-timeout.js to avoid iloop - Update BigInt constructor to match new semantics (tc39/proposal-bigint#138)
|
Tests were landed in tc39/test262#1514 |
- "CannotSuspendMainAgent" feature was changed to "CanBlockIsFalse" flag - Move annex-b tests into annex-b directory - Update variable names in nonshared-int-views.js tests - Move getReport() call in nan-for-timeout.js to avoid iloop - Update BigInt constructor to match new semantics (tc39/proposal-bigint#138)
Previously, the BigInt constructor used IsSafeInteger to check if a Number
is safe to cast to a BigInt without losing information. This check is
unnecessarily conservative--it rules out large integer-valued Numbers.
This patch switches to just check that the Number is an integer value.
This patch specifies the resolution of the discussion in the March
2018 TC39 meeting.
Closes #132