Normative: Validate fractionalSecondDigits after truncation #2297
Conversation
gibson042
added
spec-text
Codecov Report
@@ Coverage Diff @@
## main #2297 +/- ##
==========================================
- Coverage 91.08% 91.08% -0.01%
==========================================
Files 19 19
Lines 10558 10566 +8
Branches 1695 1695
==========================================
+ Hits 9617 9624 +7
- Misses 931 932 +1
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
polyfill/lib/ecmascript.mjs
Outdated
| @@ -749,10 +749,10 @@ export const ES = ObjectAssign({}, ES2020, { | |||
| if (digits === 'auto') return { precision: 'auto', unit: 'nanosecond', increment: 1 }; | |||
| throw new RangeError(`fractionalSecondDigits must be 'auto' or 0 through 9, not ${digits}`); | |||
| } | |||
| if (NumberIsNaN(digits) || digits < 0 || digits > 9) { | |||
| const precision = ES.ToInteger(digits); | |||
| if (!NumberIsFinite(digits) || precision < 0 || precision > 9) { | |||
There was a problem hiding this comment.
Is the NumberIsFinite check needed?
There was a problem hiding this comment.
It could be NumberIsNaN instead, but this aligns better with the spec text.
There was a problem hiding this comment.
Pretty sure ToInteger should return zero for NaN
There was a problem hiding this comment.
ES.ToInteger(NaN) does return 0, but note that the NumberIsFinite check is against its input rather than its output. The spec text here calls for a RangeError when fractionalDigitsVal is not finite or ToIntegerOrInfinity(fractionalDigitsVal) is out of bounds, which is equivalent to this code in the polyfill that throws a RangeError when digits is not finite or ES.ToInteger(digits) is out of bounds.
There was a problem hiding this comment.
Eek. I think I'd prefer moving the NumberIsFinite call before the ToInteger call, then. Also, I think we can use MathTrunc instead of ToInteger.
There was a problem hiding this comment.
Do we want to present this in July or do we need to wait until tc39/ecma402#691 is discussed in TG2?
|
I have a preference for discussing it in July no matter what, but would be willing to wait if someone feels strongly about it. |
polyfill/lib/ecmascript.mjs
Outdated
| @@ -749,10 +749,10 @@ export const ES = ObjectAssign({}, ES2020, { | |||
| if (digits === 'auto') return { precision: 'auto', unit: 'nanosecond', increment: 1 }; | |||
| throw new RangeError(`fractionalSecondDigits must be 'auto' or 0 through 9, not ${digits}`); | |||
| } | |||
| if (NumberIsNaN(digits) || digits < 0 || digits > 9) { | |||
| const precision = ES.ToInteger(digits); | |||
| if (!NumberIsFinite(digits) || precision < 0 || precision > 9) { | |||
There was a problem hiding this comment.
Eek. I think I'd prefer moving the NumberIsFinite call before the ToInteger call, then. Also, I think we can use MathTrunc instead of ToInteger.
Co-authored-by: Ms2ger <Ms2ger@gmail.com>
…nalSecondDigits This implements the normative change in tc39/proposal-temporal#2297 which reached consensus at the July 2022 TC39 meeting. Values given as the fractionalSecondDigits option are now truncated to integers before they are compared to the allowable range.
|
This change reached consensus at the July 2022 TC39 meeting. Tests are in tc39/test262#3621 |
…nalSecondDigits This implements the normative change in tc39/proposal-temporal#2297 which reached consensus at the July 2022 TC39 meeting. Values given as the fractionalSecondDigits option are now truncated to integers before they are compared to the allowable range.
Validate fractionalSecondDigits after truncation tc39/proposal-temporal#2297 Spec text: https://tc39.es/proposal-temporal/#sec-temporal-tosecondsstringprecision Bug: v8:11544 Change-Id: I648f087f4fa2cfd6245c7946cfa625a7c5e3b3b9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3855702 Commit-Queue: Frank Tang <ftang@chromium.org> Reviewed-by: Shu-yu Guo <syg@chromium.org> Cr-Commit-Position: refs/heads/main@{#82801}
Fixes #2296
The result is consistent with my ECMA-402 proposal at tc39/ecma402#691 (reject non-numeric input and perform validation of numeric input after truncation).