Changeset 282257 in webkit

Timestamp:

Sep 9, 2021 8:23:20 PM (10 months ago)

Author:

ysuzuki@apple.com

Message:

[JSC] Intl.Locale weekendInfo should list all weekend days instead of range
​https://bugs.webkit.org/show_bug.cgi?id=230108

Reviewed by Ross Kirsling.

JSTests:

• stress/intl-locale-info.js:

(throw.new.Error):
(let.enGB.new.Intl.Locale.shouldBe): Deleted.

• test262/config.yaml:

Source/JavaScriptCore:

We cannot assume that weekend is contiguous. For example, Burnei's weekend is Friday and Sunday.
This is raised in [1], and our conclusion in Sep-9 meeting is that we should have an array which
includes all the weekend days. The change is merged in [2], and this patch changes our implementation
accordingly.

[1]: ​https://github.com/tc39/proposal-intl-locale-info/issues/25
[2]: ​https://github.com/tc39/proposal-intl-locale-info/commit/afb1e269dd698476a2514129235cdad88af60e6f

• runtime/IntlLocale.cpp:

(JSC::IntlLocale::weekInfo):

• runtime/IntlObjectInlines.h:

(JSC::createArrayFromIntVector):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/intl-locale-info.js (modified) (1 diff)
• JSTests/test262/config.yaml (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/IntlLocale.cpp (modified) (2 diffs)
• Source/JavaScriptCore/runtime/IntlObjectInlines.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2021-09-09  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Intl.Locale weekendInfo should list all weekend days instead of range
+        https://bugs.webkit.org/show_bug.cgi?id=230108
+
+        Reviewed by Ross Kirsling.
+
+        * stress/intl-locale-info.js:
+        (throw.new.Error):
+        (let.enGB.new.Intl.Locale.shouldBe): Deleted.
+        * test262/config.yaml:
+
 2021-09-09  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/JSTests/stress/intl-locale-info.js

@@ -6 +6 @@
 {
     let he = new Intl.Locale("he")
-    shouldBe(JSON.stringify(he.weekInfo), `{"firstDay":7,"weekendStart":5,"weekendEnd":6,"minimalDays":1}`);
+    shouldBe(JSON.stringify(he.weekInfo), `{"firstDay":7,"weekend":[5,6],"minimalDays":1}`);
     let af = new Intl.Locale("af")
-    shouldBe(JSON.stringify(af.weekInfo), `{"firstDay":7,"weekendStart":6,"weekendEnd":7,"minimalDays":1}`);
+    shouldBe(JSON.stringify(af.weekInfo), `{"firstDay":7,"weekend":[6,7],"minimalDays":1}`);
     let enGB = new Intl.Locale("en-GB")
-    shouldBe(JSON.stringify(enGB.weekInfo), `{"firstDay":1,"weekendStart":6,"weekendEnd":7,"minimalDays":4}`);
+    shouldBe(JSON.stringify(enGB.weekInfo), `{"firstDay":1,"weekend":[6,7],"minimalDays":4}`);
+    let msBN = new Intl.Locale("ms-BN");
+    // "weekend" should be [5,7]. But currently ICU/CLDR does not support representing non-contiguous weekend.
+    shouldBe(JSON.stringify(msBN.weekInfo), `{"firstDay":1,"weekend":[6,7],"minimalDays":1}`);
 }
 {

trunk/JSTests/test262/config.yaml

@@ -72 +72 @@
     - test/intl402/Locale/constructor-non-iana-canon.js
     - test/intl402/Locale/likely-subtags.js
+
+    # The spec is changed, but the test is not updated yet. https://github.com/tc39/proposal-intl-locale-info/pull/44
+    - test/intl402/Locale/prototype/weekInfo/output-object-keys.js
 
     # Pass only in ICU 68~

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-09-09  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Intl.Locale weekendInfo should list all weekend days instead of range
+        https://bugs.webkit.org/show_bug.cgi?id=230108
+
+        Reviewed by Ross Kirsling.
+
+        We cannot assume that weekend is contiguous. For example, Burnei's weekend is Friday and Sunday.
+        This is raised in [1], and our conclusion in Sep-9 meeting is that we should have an array which
+        includes all the weekend days. The change is merged in [2], and this patch changes our implementation
+        accordingly.
+
+        [1]: https://github.com/tc39/proposal-intl-locale-info/issues/25
+        [2]: https://github.com/tc39/proposal-intl-locale-info/commit/afb1e269dd698476a2514129235cdad88af60e6f
+
+        * runtime/IntlLocale.cpp:
+        (JSC::IntlLocale::weekInfo):
+        * runtime/IntlObjectInlines.h:
+        (JSC::createArrayFromIntVector):
+
 2021-09-09  Darin Adler  <darin@apple.com>
 

trunk/Source/JavaScriptCore/runtime/IntlLocale.cpp

@@ -793 +793 @@
     };
 
-    static_assert(UCAL_SUNDAY == 1);
-    static_assert(UCAL_SATURDAY == 7);
-    UCalendarWeekdayType previous = canonicalizeDayOfWeekType(ucal_getDayOfWeekType(calendar.get(), UCAL_SATURDAY, &status));
-    if (!U_SUCCESS(status)) {
-        throwTypeError(globalObject, scope, "invalid locale"_s);
-        return nullptr;
-    }
-
-    int32_t weekendStart = 0;
-    int32_t weekendEnd = 0;
-    for (int32_t day = UCAL_SUNDAY; day <= UCAL_SATURDAY; ++day) {
-        UCalendarWeekdayType type = canonicalizeDayOfWeekType(ucal_getDayOfWeekType(calendar.get(), static_cast<UCalendarDaysOfWeek>(day), &status));
-        if (!U_SUCCESS(status)) {
-            throwTypeError(globalObject, scope, "invalid locale"_s);
-            return nullptr;
-        }
-        if (previous != type) {
-            switch (type) {
-            case UCAL_WEEKDAY: // WeekEnd => WeekDay
-                if (day == UCAL_SUNDAY)
-                    weekendEnd = UCAL_SATURDAY;
-                else
-                    weekendEnd = day - 1;
-                break;
-            case UCAL_WEEKEND: // WeekDay => WeekEnd
-                weekendStart = day;
-                break;
-            default:
-                ASSERT_NOT_REACHED();
-                break;
-            }
-        }
-        previous = type;
-    }
+    auto convertMondayBasedDayToUCalendarDaysOfWeek = [](int32_t day) -> UCalendarDaysOfWeek {
+        // Convert from
+        //     Monday => 1
+        //     Sunday => 7
+        // to
+        //     Sunday => 1
+        //     Saturday => 7
+        static_assert(UCAL_SUNDAY == 1);
+        static_assert(UCAL_SATURDAY == 7);
+        if (day == 7)
+            return UCAL_SUNDAY;
+        return static_cast<UCalendarDaysOfWeek>(day + 1);
+    };
 
     auto convertUCalendarDaysOfWeekToMondayBasedDay = [](int32_t day) -> int32_t {
@@ -840 +819 @@
     };
 
+    Vector<int32_t, 7> weekend;
+    for (int32_t day = 1; day <= 7; ++day) {
+        UCalendarWeekdayType type = canonicalizeDayOfWeekType(ucal_getDayOfWeekType(calendar.get(), convertMondayBasedDayToUCalendarDaysOfWeek(day), &status));
+        if (!U_SUCCESS(status)) {
+            throwTypeError(globalObject, scope, "invalid locale"_s);
+            return nullptr;
+        }
+        switch (type) {
+        case UCAL_WEEKDAY:
+            break;
+        case UCAL_WEEKEND:
+            weekend.append(day);
+            break;
+        default:
+            ASSERT_NOT_REACHED();
+            break;
+        }
+    }
+
+    auto* weekendArray = createArrayFromIntVector(globalObject, WTFMove(weekend));
+    RETURN_IF_EXCEPTION(scope, { });
+
     JSObject* result = constructEmptyObject(globalObject);
     result->putDirect(vm, Identifier::fromString(vm, "firstDay"), jsNumber(convertUCalendarDaysOfWeekToMondayBasedDay(firstDayOfWeek)));
-    result->putDirect(vm, Identifier::fromString(vm, "weekendStart"), jsNumber(convertUCalendarDaysOfWeekToMondayBasedDay(weekendStart)));
-    result->putDirect(vm, Identifier::fromString(vm, "weekendEnd"), jsNumber(convertUCalendarDaysOfWeekToMondayBasedDay(weekendEnd)));
+    result->putDirect(vm, Identifier::fromString(vm, "weekend"), weekendArray);
     result->putDirect(vm, Identifier::fromString(vm, "minimalDays"), jsNumber(minimalDays));
     return result;

trunk/Source/JavaScriptCore/runtime/IntlObjectInlines.h

@@ -244 +244 @@
 }
 
+template<typename Container>
+JSArray* createArrayFromIntVector(JSGlobalObject* globalObject, const Container& elements)
+{
+    VM& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    JSArray* result = JSArray::tryCreate(vm, globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous), elements.size());
+    if (!result) {
+        throwOutOfMemoryError(globalObject, scope);
+        return nullptr;
+    }
+    for (unsigned index = 0; index < elements.size(); ++index) {
+        result->putDirectIndex(globalObject, index, jsNumber(elements[index]));
+        RETURN_IF_EXCEPTION(scope, { });
+    }
+    return result;
+}
+
 } // namespace JSC