Skip to content
Permalink
Browse files
[JSC] Adopt Intl.NumberFormat v3 spec change for useGrouping option
https://bugs.webkit.org/show_bug.cgi?id=242200
rdar://93712843

Reviewed by Ross Kirsling.

Intl.NumberFormat v3's useGrouping option has incompatibility with the previous version.
We observe it in JSTests and PerformanceTests. And we discussed it in ECMA-402 meeting[1].
ECMA-402 members agree that we fix this incompatibility, and it is done in [2].
This patch applies this change to our implementation.

[1]: https://github.com/tc39/ecma402/blob/master/meetings/notes-2022-03-17.md#web-compatibility-allow-the-string-true-for-usegrouping
[2]: tc39/proposal-intl-numberformat-v3@0cf3965

* JSTests/stress/intl-numberformat-usegrouping-v3.js:
(nonListedUseGrouping.forEach):
(invalidUseGrouping.forEach): Deleted.
* JSTests/test262/expectations.yaml:
* PerformanceTests/Skipped:
* Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:
(JSC::IntlNumberFormat::initializeNumberFormat):
* Source/JavaScriptCore/runtime/IntlObjectInlines.h:
(JSC::intlStringOrBooleanOption):

Canonical link: https://commits.webkit.org/252013@main
  • Loading branch information
Constellation committed Jun 30, 2022
1 parent d224393 commit 70694b2a29d9e4233af0341e709582086740785d
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 24 deletions.
@@ -52,19 +52,20 @@ let validUseGrouping = [
false,
];

let invalidUseGrouping = [
let nonListedUseGrouping = [
"min-2",
"true",
"false",
];

validUseGrouping.forEach(function(useGrouping) {
let nf = new Intl.NumberFormat(undefined, {useGrouping});
shouldBe(useGrouping, nf.resolvedOptions().useGrouping);
});

invalidUseGrouping.forEach(function(useGrouping) {
shouldThrow(() => {
let nf = new Intl.NumberFormat(undefined, {useGrouping}); }, `RangeError: useGrouping must be either true, false, "min2", "auto", or "always"`);
nonListedUseGrouping.forEach(function(useGrouping) {
let nf = new Intl.NumberFormat(undefined, {useGrouping});
shouldBe(nf.resolvedOptions().useGrouping, `auto`);
});

// useGrouping: undefined get "auto"
@@ -1242,6 +1242,9 @@ test/intl402/PluralRules/prototype/selectRange/prop-desc.js:
test/intl402/PluralRules/prototype/selectRange/x-greater-than-y-throws.js:
default: 'Test262Error: Expected a RangeError but got a TypeError'
strict mode: 'Test262Error: Expected a RangeError but got a TypeError'
test/intl402/NumberFormat/test-option-useGrouping-extended.js:
default: 'Test262Error: "undefined" Expected a RangeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: "undefined" Expected a RangeError to be thrown but no exception was thrown at all'
test/intl402/Temporal/Duration/compare/relativeto-hour.js:
default: "TypeError: undefined is not an object (evaluating 'Temporal.ZonedDateTime.from')"
strict mode: "TypeError: undefined is not an object (evaluating 'Temporal.ZonedDateTime.from')"
@@ -152,8 +152,5 @@ MediaTime/
[Win] IndexedDB/stress/large-number-of-inserts.html
[Win] IndexedDB/stress/large-string-keys.html

# https://bugs.webkit.org/show_bug.cgi?id=240767
Intl/numberformat-create-all-options.html

# https://bugs.webkit.org/show_bug.cgi?id=241729
Layout/nested-flexbox.html
@@ -414,7 +414,7 @@ void IntlNumberFormat::initializeNumberFormat(JSGlobalObject* globalObject, JSVa
if (m_notation == IntlNotation::Compact)
defaultUseGrouping = UseGrouping::Min2;

m_useGrouping = intlStringOrBooleanOption<UseGrouping>(globalObject, options, Identifier::fromString(vm, "useGrouping"_s), UseGrouping::Always, UseGrouping::False, { { "min2"_s, UseGrouping::Min2 }, { "auto"_s, UseGrouping::Auto }, { "always"_s, UseGrouping::Always } }, "useGrouping must be either true, false, \"min2\", \"auto\", or \"always\""_s, defaultUseGrouping);
m_useGrouping = intlStringOrBooleanOption<UseGrouping>(globalObject, options, Identifier::fromString(vm, "useGrouping"_s), UseGrouping::Always, UseGrouping::False, { { "min2"_s, UseGrouping::Min2 }, { "auto"_s, UseGrouping::Auto }, { "always"_s, UseGrouping::Always } }, defaultUseGrouping);
RETURN_IF_EXCEPTION(scope, void());

m_signDisplay = intlOption<SignDisplay>(globalObject, options, Identifier::fromString(vm, "signDisplay"_s), { { "auto"_s, SignDisplay::Auto }, { "never"_s, SignDisplay::Never }, { "always"_s, SignDisplay::Always }, { "exceptZero"_s, SignDisplay::ExceptZero }, { "negative"_s, SignDisplay::Negative } }, "signDisplay must be either \"auto\", \"never\", \"always\", \"exceptZero\", or \"negative\""_s, SignDisplay::Auto);
@@ -152,7 +152,7 @@ ResultType intlOption(JSGlobalObject* globalObject, JSObject* options, PropertyN
}

template<typename ResultType>
ResultType intlStringOrBooleanOption(JSGlobalObject* globalObject, JSObject* options, PropertyName property, ResultType trueValue, ResultType falsyValue, std::initializer_list<std::pair<ASCIILiteral, ResultType>> values, ASCIILiteral notFoundMessage, ResultType fallback)
ResultType intlStringOrBooleanOption(JSGlobalObject* globalObject, JSObject* options, PropertyName property, ResultType trueValue, ResultType falsyValue, std::initializer_list<std::pair<ASCIILiteral, ResultType>> values, ResultType fallback)
{
// https://tc39.es/proposal-intl-numberformat-v3/out/negotiation/diff.html#sec-getstringorbooleanoption

@@ -167,25 +167,24 @@ ResultType intlStringOrBooleanOption(JSGlobalObject* globalObject, JSObject* opt
JSValue value = options->get(globalObject, property);
RETURN_IF_EXCEPTION(scope, { });

if (!value.isUndefined()) {
if (value.isBoolean() && value.asBoolean())
return trueValue;
if (value.isUndefined())
return fallback;

bool valueBoolean = value.toBoolean(globalObject);
RETURN_IF_EXCEPTION(scope, { });
if (value.isBoolean() && value.asBoolean())
return trueValue;

if (!valueBoolean)
return falsyValue;
bool valueBoolean = value.toBoolean(globalObject);
RETURN_IF_EXCEPTION(scope, { });

String stringValue = value.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, { });
if (!valueBoolean)
return falsyValue;

for (const auto& entry : values) {
if (entry.first == stringValue)
return entry.second;
}
throwException(globalObject, scope, createRangeError(globalObject, notFoundMessage));
return { };
String stringValue = value.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, { });

for (const auto& entry : values) {
if (entry.first == stringValue)
return entry.second;
}

return fallback;

0 comments on commit 70694b2

Please sign in to comment.