Skip to content

Commit

Permalink
[intl] Sync to ECMA402 PR811
Browse files Browse the repository at this point in the history 
Fix order of rounding* option reads and resolvedOptions() of NumberFormat and PluralRules

tc39/ecma402#811

Details:

Change order of rounding* option reads and resolvedOptions()
1. Move the reading of "roundingPriority" from before reading "roundingIncrement" to after reading "roundingMode" so these three fields are read in alphabetic order. https://tc39.es/ecma402/#sec-setnfdigitoptions

2. Move the output order of "roundingIncrement" from after output "roundingMode" to before that so these two fields are read in alphabetic order in the resolvedOptions of both Intl.NumberFormat and Intl.PluralRules. https://tc39.es/ecma402/#sec-intl.numberformat.prototype.resolvedoptions https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions

3. Move the output of "roundingPriority" from after "trailingZeroDisplay" before that to sync w/ Intl.NumberFormat after PR768. https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions

4. Move the output of "pluralCategories" from after "trailingZeroDisplay" to before "roundingIncrement" so these options will be in alphabetic order. https://tc39.es/ecma402/#sec-intl.pluralrules.prototype.resolvedoptions


Bug: v8:14308
Change-Id: I37168bf0a6e03c0459c723c98feb8b34d95413fa
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4855044
Reviewed-by: Shu-yu Guo <[email protected]>
Commit-Queue: Frank Tang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#90319}
  • Loading branch information
@FrankYFTang
FrankYFTang authored and V8 LUCI CQ committed Oct 10, 2023
1 parent ea996ad commit 37dee47
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 39 deletions.
32 changes: 16 additions & 16 deletions src/objects/intl-objects.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1597,27 +1597,14 @@ Maybe<Intl::NumberFormatDigitOptions> Intl::SetNumberFormatDigitOptions(
// 6. Set intlObj.[[MinimumIntegerDigits]] to mnid.
digit_options.minimum_integer_digits = mnid;

// 7. Let roundingPriority be ? GetOption(options, "roundingPriority",
// "string", « "auto", "morePrecision", "lessPrecision" », "auto").

Maybe<RoundingPriority> maybe_rounding_priority =
GetStringOption<RoundingPriority>(
isolate, options, "roundingPriority", service,
{"auto", "morePrecision", "lessPrecision"},
{RoundingPriority::kAuto, RoundingPriority::kMorePrecision,
RoundingPriority::kLessPrecision},
RoundingPriority::kAuto);
MAYBE_RETURN(maybe_rounding_priority, Nothing<NumberFormatDigitOptions>());
digit_options.rounding_priority = maybe_rounding_priority.FromJust();

// 8. Let roundingIncrement be ? GetNumberOption(options, "roundingIncrement",
// 7. Let roundingIncrement be ? GetNumberOption(options, "roundingIncrement",
// 1, 5000, 1).
Maybe<int> maybe_rounding_increment = GetNumberOption(
isolate, options, factory->roundingIncrement_string(), 1, 5000, 1);
if (!maybe_rounding_increment.To(&digit_options.rounding_increment)) {
return Nothing<NumberFormatDigitOptions>();
}
// 9. If roundingIncrement is not in « 1, 2, 5, 10, 20, 25, 50, 100, 200, 250,
// 8. If roundingIncrement is not in « 1, 2, 5, 10, 20, 25, 50, 100, 200, 250,
// 500, 1000, 2000, 2500, 5000 », throw a RangeError exception.
if (!IsValidRoundingIncrement(digit_options.rounding_increment)) {
THROW_NEW_ERROR_RETURN_VALUE(
Expand All @@ -1627,7 +1614,7 @@ Maybe<Intl::NumberFormatDigitOptions> Intl::SetNumberFormatDigitOptions(
Nothing<NumberFormatDigitOptions>());
}

// 10. Let roundingMode be ? GetOption(options, "roundingMode", string, «
// 9. Let roundingMode be ? GetOption(options, "roundingMode", string, «
// "ceil", "floor", "expand", "trunc", "halfCeil", "halfFloor", "halfExpand",
// "halfTrunc", "halfEven" », "halfExpand").
Maybe<RoundingMode> maybe_rounding_mode = GetStringOption<RoundingMode>(
Expand All @@ -1642,6 +1629,19 @@ Maybe<Intl::NumberFormatDigitOptions> Intl::SetNumberFormatDigitOptions(
MAYBE_RETURN(maybe_rounding_mode, Nothing<NumberFormatDigitOptions>());
digit_options.rounding_mode = maybe_rounding_mode.FromJust();

// 10. Let roundingPriority be ? GetOption(options, "roundingPriority",
// "string", « "auto", "morePrecision", "lessPrecision" », "auto").

Maybe<RoundingPriority> maybe_rounding_priority =
GetStringOption<RoundingPriority>(
isolate, options, "roundingPriority", service,
{"auto", "morePrecision", "lessPrecision"},
{RoundingPriority::kAuto, RoundingPriority::kMorePrecision,
RoundingPriority::kLessPrecision},
RoundingPriority::kAuto);
MAYBE_RETURN(maybe_rounding_priority, Nothing<NumberFormatDigitOptions>());
digit_options.rounding_priority = maybe_rounding_priority.FromJust();

// 11. Let trailingZeroDisplay be ? GetOption(options, "trailingZeroDisplay",
// string, « "auto", "stripIfInteger" », "auto").
Maybe<TrailingZeroDisplay> maybe_trailing_zero_display =
Expand Down
31 changes: 16 additions & 15 deletions src/objects/js-number-format.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -556,9 +556,11 @@ Handle<String> SignDisplayString(Isolate* isolate,
return ReadOnlyRoots(isolate).auto_string_handle();
}

} // anonymous namespace

// Return RoundingMode as string based on skeleton.
Handle<String> RoundingModeString(Isolate* isolate,
const icu::UnicodeString& skeleton) {
Handle<String> JSNumberFormat::RoundingModeString(
Isolate* isolate, const icu::UnicodeString& skeleton) {
static const char* rounding_mode = "rounding-mode-";
int32_t start = skeleton.indexOf(rounding_mode);
if (start >= 0) {
Expand Down Expand Up @@ -611,8 +613,8 @@ Handle<String> RoundingModeString(Isolate* isolate,
return ReadOnlyRoots(isolate).halfEven_string_handle();
}

Handle<Object> RoundingIncrement(Isolate* isolate,
const icu::UnicodeString& skeleton) {
Handle<Object> JSNumberFormat::RoundingIncrement(
Isolate* isolate, const icu::UnicodeString& skeleton) {
int32_t cur = skeleton.indexOf(u"precision-increment/");
if (cur < 0) return isolate->factory()->NewNumberFromInt(1);
cur += 20; // length of "precision-increment/"
Expand All @@ -627,8 +629,8 @@ Handle<Object> RoundingIncrement(Isolate* isolate,
}

// Return RoundingPriority as string based on skeleton.
Handle<String> RoundingPriorityString(Isolate* isolate,
const icu::UnicodeString& skeleton) {
Handle<String> JSNumberFormat::RoundingPriorityString(
Isolate* isolate, const icu::UnicodeString& skeleton) {
int32_t found;
// If #r or @r is followed by a SPACE or in the end of line.
if ((found = skeleton.indexOf("#r")) >= 0 ||
Expand All @@ -648,8 +650,8 @@ Handle<String> RoundingPriorityString(Isolate* isolate,
}

// Return trailingZeroDisplay as string based on skeleton.
Handle<String> TrailingZeroDisplayString(Isolate* isolate,
const icu::UnicodeString& skeleton) {
Handle<String> JSNumberFormat::TrailingZeroDisplayString(
Isolate* isolate, const icu::UnicodeString& skeleton) {
int32_t found;
if ((found = skeleton.indexOf("/w")) >= 0) {
if (found + 2 == skeleton.length() || skeleton[found + 2] == ' ') {
Expand All @@ -659,8 +661,6 @@ Handle<String> TrailingZeroDisplayString(Isolate* isolate,
return ReadOnlyRoots(isolate).auto_string_handle();
}

} // anonymous namespace

// Return the minimum integer digits by counting the number of '0' after
// "integer-width/*" in the skeleton.
// Ex: Return 15 for skeleton as
Expand Down Expand Up @@ -910,8 +910,9 @@ Handle<JSObject> JSNumberFormat::ResolvedOptions(
// [[Notation]] "notation"
// [[CompactDisplay]] "compactDisplay"
// [[SignDisplay]] "signDisplay"
// [[RoundingMode]] "roundingMode"
// [[RoundingIncrement]] "roundingIncrement"
// [[RoundingMode]] "roundingMode"
// [[ComputedRoundingPriority]] "roundingPriority"
// [[TrailingZeroDisplay]] "trailingZeroDisplay"

CHECK(JSReceiver::CreateDataProperty(isolate, options,
Expand Down Expand Up @@ -1016,14 +1017,14 @@ Handle<JSObject> JSNumberFormat::ResolvedOptions(
isolate, options, factory->signDisplay_string(),
SignDisplayString(isolate, skeleton), Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, options, factory->roundingMode_string(),
RoundingModeString(isolate, skeleton), Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, options, factory->roundingIncrement_string(),
RoundingIncrement(isolate, skeleton), Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, options, factory->roundingMode_string(),
RoundingModeString(isolate, skeleton), Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, options, factory->roundingPriority_string(),
RoundingPriorityString(isolate, skeleton), Just(kDontThrow))
Expand Down
9 changes: 9 additions & 0 deletions src/objects/js-number-format.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@ class JSNumberFormat
static bool SignificantDigitsFromSkeleton(const icu::UnicodeString& skeleton,
int32_t* minimum, int32_t* maximum);

static Handle<String> RoundingModeString(Isolate* isolate,
const icu::UnicodeString& skeleton);
static Handle<String> RoundingPriorityString(
Isolate* isolate, const icu::UnicodeString& skeleton);
static Handle<String> TrailingZeroDisplayString(
Isolate* isolate, const icu::UnicodeString& skeleton);
static Handle<Object> RoundingIncrement(Isolate* isolate,
const icu::UnicodeString& skeleton);

enum class ShowTrailingZeros { kShow, kHide };

static icu::number::UnlocalizedNumberFormatter SetDigitOptionsToFormatter(
Expand Down
29 changes: 25 additions & 4 deletions src/objects/js-plural-rules.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,26 +291,47 @@ Handle<JSObject> JSPluralRules::ResolvedOptions(
int32_t count = categories->count(status);
DCHECK(U_SUCCESS(status));

Handle<FixedArray> plural_categories =
isolate->factory()->NewFixedArray(count);
Factory* factory = isolate->factory();
Handle<FixedArray> plural_categories = factory->NewFixedArray(count);
for (int32_t i = 0; i < count; i++) {
const icu::UnicodeString* category = categories->snext(status);
DCHECK(U_SUCCESS(status));
if (category == nullptr) break;

std::string keyword;
Handle<String> value = isolate->factory()->NewStringFromAsciiChecked(
Handle<String> value = factory->NewStringFromAsciiChecked(
category->toUTF8String(keyword).data());
plural_categories->set(i, *value);
}

// 7. Perform ! CreateDataProperty(options, "pluralCategories",
// CreateArrayFromList(pluralCategories)).
Handle<JSArray> plural_categories_value =
isolate->factory()->NewJSArrayWithElements(plural_categories);
factory->NewJSArrayWithElements(plural_categories);
CreateDataPropertyForOptions(isolate, options, plural_categories_value,
"pluralCategories");

CHECK(JSReceiver::CreateDataProperty(
isolate, options, factory->roundingIncrement_string(),
JSNumberFormat::RoundingIncrement(isolate, skeleton),
Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, options, factory->roundingMode_string(),
JSNumberFormat::RoundingModeString(isolate, skeleton),
Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, options, factory->roundingPriority_string(),
JSNumberFormat::RoundingPriorityString(isolate, skeleton),
Just(kDontThrow))
.FromJust());
CHECK(JSReceiver::CreateDataProperty(
isolate, options, factory->trailingZeroDisplay_string(),
JSNumberFormat::TrailingZeroDisplayString(isolate, skeleton),
Just(kDontThrow))
.FromJust());

return options;
}

Expand Down
2 changes: 1 addition & 1 deletion test/intl/number-format/resolved-options-order.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ assertEquals(
"useGrouping," +
"notation," +
"signDisplay," +
"roundingMode," +
"roundingIncrement," +
"roundingMode," +
"roundingPriority," +
"trailingZeroDisplay",
Object.keys((new Intl.NumberFormat("en")).resolvedOptions()).join(","));
4 changes: 1 addition & 3 deletions test/test262/test262.status
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2139,10 +2139,8 @@
'intl402/DateTimeFormat/prototype/resolvedOptions/offset-timezone-basic': [FAIL],
'intl402/DateTimeFormat/prototype/resolvedOptions/offset-timezone-change': [FAIL],

# https://bugs.chromium.org/p/v8/issues/detail?id=14308
'intl402/NumberFormat/constructor-option-read-order': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=14315
'intl402/NumberFormat/prototype/resolvedOptions/return-keys-order-default': [FAIL],
'intl402/PluralRules/constructor-option-read-order': [FAIL],

############################ INVALID TESTS #############################

Expand Down

0 comments on commit 37dee47

Please sign in to comment.