[Intl] Fixes Intl.DateTimeFormat hourCycle option issues.

Add bit flags to remember hourCycle
Reorder the code in JSDateTimeFormat::Initialize
Implement the hourCycle option resolutions
Fix intl402/DateTimeFormat/prototype/resolvedOptions/hourCycle in test262

Bug: v8:7482
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Idc136276da89b95df6ae864161b114e34f9dcae8
Reviewed-on: https://chromium-review.googlesource.com/c/1253101
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58233}

src/objects-debug.cc

@@ -2065,6 +2065,7 @@ void JSDateTimeFormat::JSDateTimeFormatVerify(Isolate* isolate) {
   VerifyObjectField(isolate, kICULocaleOffset);
   VerifyObjectField(isolate, kICUSimpleDateFormatOffset);
   VerifyObjectField(isolate, kBoundFormatOffset);
+  VerifyObjectField(isolate, kFlagsOffset);
 }
 
 void JSListFormat::JSListFormatVerify(Isolate* isolate) {

src/objects-printer.cc

@@ -2037,6 +2037,7 @@ void JSDateTimeFormat::JSDateTimeFormatPrint(std::ostream& os) {  // NOLINT
   os << "\n - icu locale: " << Brief(icu_locale());
   os << "\n - icu simple date format: " << Brief(icu_simple_date_format());
   os << "\n - bound format: " << Brief(bound_format());
+  os << "\n - hour cycle: " << HourCycleAsString();
   JSObjectPrintBody(os, *this);
 }
 

src/objects/intl-objects.cc

@@ -1849,5 +1849,14 @@ Maybe<Intl::MatcherOption> Intl::GetLocaleMatcher(Isolate* isolate,
       {Intl::MatcherOption::kLookup, Intl::MatcherOption::kBestFit},
       Intl::MatcherOption::kLookup);
 }
+
+Intl::HourCycle Intl::ToHourCycle(const std::string& hc) {
+  if (hc == "h11") return Intl::HourCycle::kH11;
+  if (hc == "h12") return Intl::HourCycle::kH12;
+  if (hc == "h23") return Intl::HourCycle::kH23;
+  if (hc == "h24") return Intl::HourCycle::kH24;
+  return Intl::HourCycle::kUndefined;
+}
+
 }  // namespace internal
 }  // namespace v8

src/objects/intl-objects.h

@@ -223,6 +223,8 @@ class Intl {
   // enum for "hourCycle" option: shared by Intl.Locale and Intl.DateTimeFormat.
   enum class HourCycle { kH11, kH12, kH23, kH24, kUndefined };
 
+  static HourCycle ToHourCycle(const std::string& str);
+
   // Shared function to read the "hourCycle" option.
   V8_WARN_UNUSED_RESULT static Maybe<HourCycle> GetHourCycle(
       Isolate* isolate, Handle<JSReceiver> options, const char* method);

src/objects/js-date-time-format-inl.h

@@ -24,6 +24,17 @@ ACCESSORS(JSDateTimeFormat, icu_locale, Managed<icu::Locale>, kICULocaleOffset);
 ACCESSORS(JSDateTimeFormat, icu_simple_date_format,
           Managed<icu::SimpleDateFormat>, kICUSimpleDateFormatOffset)
 ACCESSORS(JSDateTimeFormat, bound_format, Object, kBoundFormatOffset);
+SMI_ACCESSORS(JSDateTimeFormat, flags, kFlagsOffset)
+
+inline void JSDateTimeFormat::set_hour_cycle(Intl::HourCycle hour_cycle) {
+  int hints = flags();
+  hints = HourCycleBits::update(hints, hour_cycle);
+  set_flags(hints);
+}
+
+inline Intl::HourCycle JSDateTimeFormat::hour_cycle() const {
+  return HourCycleBits::decode(flags());
+}
 
 CAST_ACCESSOR2(JSDateTimeFormat);
 

src/objects/js-date-time-format.cc

@@ -92,7 +92,11 @@ const std::vector<PatternItem> GetPatternItems() {
                   {{"HH", "2-digit"},
                    {"H", "numeric"},
                    {"hh", "2-digit"},
-                   {"h", "numeric"}},
+                   {"h", "numeric"},
+                   {"kk", "2-digit"},
+                   {"k", "numeric"},
+                   {"KK", "2-digit"},
+                   {"K", "numeric"}},
                   k2DigitNumeric),
       PatternItem("minute", {{"mm", "2-digit"}, {"m", "numeric"}},
                   k2DigitNumeric),
@@ -119,12 +123,6 @@ class PatternData {
   std::vector<const char*> allowed_values;
 };
 
-enum HourOption {
-  H_UNKNOWN,
-  H_12,
-  H_24,
-};
-
 const std::vector<PatternData> CreateCommonData(const PatternData& hour_data) {
   std::vector<PatternData> build;
   for (const PatternItem& item : GetPatternItems()) {
@@ -145,17 +143,36 @@ const std::vector<PatternData> CreateData(const char* digit2,
                   {"2-digit", "numeric"}));
 }
 
-const std::vector<PatternData> GetPatternData(HourOption option) {
+// According to "Date Field Symbol Table" in
+// http://userguide.icu-project.org/formatparse/datetime
+// Symbol | Meaning              | Example(s)
+//   h      hour in am/pm (1~12)    h    7
+//                                  hh   07
+//   H      hour in day (0~23)      H    0
+//                                  HH   00
+//   k      hour in day (1~24)      k    24
+//                                  kk   24
+//   K      hour in am/pm (0~11)    K    0
+//                                  KK   00
+const std::vector<PatternData> GetPatternData(Intl::HourCycle hour_cycle) {
   const std::vector<PatternData> data = CreateData("jj", "j");
+  const std::vector<PatternData> data_h11 = CreateData("KK", "K");
   const std::vector<PatternData> data_h12 = CreateData("hh", "h");
-  const std::vector<PatternData> data_h24 = CreateData("HH", "H");
-  switch (option) {
-    case HourOption::H_12:
+  const std::vector<PatternData> data_h23 = CreateData("HH", "H");
+  const std::vector<PatternData> data_h24 = CreateData("kk", "k");
+  switch (hour_cycle) {
+    case Intl::HourCycle::kH11:
+      return data_h11;
+    case Intl::HourCycle::kH12:
       return data_h12;
-    case HourOption::H_24:
+    case Intl::HourCycle::kH23:
+      return data_h23;
+    case Intl::HourCycle::kH24:
       return data_h24;
-    case HourOption::H_UNKNOWN:
+    case Intl::HourCycle::kUndefined:
       return data;
+    default:
+      UNREACHABLE();
   }
 }
 
@@ -265,6 +282,8 @@ MaybeHandle<JSObject> JSDateTimeFormat::ResolvedOptions(
 
   Handle<Object> resolved_obj;
 
+  CHECK_NOT_NULL(date_time_format->icu_locale());
+  CHECK_NOT_NULL(date_time_format->icu_locale()->raw());
   UErrorCode status = U_ZERO_ERROR;
   char language[ULOC_FULLNAME_CAPACITY];
   uloc_toLanguageTag(date_time_format->icu_locale()->raw()->getName(), language,
@@ -367,50 +386,41 @@ MaybeHandle<JSObject> JSDateTimeFormat::ResolvedOptions(
                                        factory->timeZone_string(),
                                        timezone_value, kDontThrow)
             .FromJust());
-  if (pattern.find('h') != std::string::npos) {
-    CHECK(JSReceiver::CreateDataProperty(isolate, options,
-                                         factory->hourCycle_string(),
-                                         factory->h11_string(), kDontThrow)
-              .FromJust());
-  } else if (pattern.find('H') != std::string::npos) {
-    CHECK(JSReceiver::CreateDataProperty(isolate, options,
-                                         factory->hourCycle_string(),
-                                         factory->h12_string(), kDontThrow)
-              .FromJust());
-  } else if (pattern.find('k') != std::string::npos) {
-    CHECK(JSReceiver::CreateDataProperty(isolate, options,
-                                         factory->hourCycle_string(),
-                                         factory->h23_string(), kDontThrow)
-              .FromJust());
-  } else if (pattern.find('K') != std::string::npos) {
-    CHECK(JSReceiver::CreateDataProperty(isolate, options,
-                                         factory->hourCycle_string(),
-                                         factory->h24_string(), kDontThrow)
-              .FromJust());
-  }
-  // b. If p is "hour12", then
-  //  i. Let hc be dtf.[[HourCycle]].
-  //  ii. If hc is "h11" or "h12", let v be true.
-  //  iii. Else if, hc is "h23" or "h24", let v be false.
-  //  iv. Else, let v be undefined.
-  if (pattern.find('h') != std::string::npos) {
-    CHECK(JSReceiver::CreateDataProperty(isolate, options,
-                                         factory->hour12_string(),
-                                         factory->true_value(), kDontThrow)
-              .FromJust());
-  } else if (pattern.find('H') != std::string::npos) {
-    CHECK(JSReceiver::CreateDataProperty(isolate, options,
-                                         factory->hour12_string(),
-                                         factory->false_value(), kDontThrow)
+
+  // 5.b.i. Let hc be dtf.[[HourCycle]].
+  Intl::HourCycle hc = date_time_format->hour_cycle();
+
+  if (hc != Intl::HourCycle::kUndefined) {
+    CHECK(JSReceiver::CreateDataProperty(
+              isolate, options, factory->hourCycle_string(),
+              date_time_format->HourCycleAsString(), kDontThrow)
               .FromJust());
+    switch (hc) {
+      //  ii. If hc is "h11" or "h12", let v be true.
+      case Intl::HourCycle::kH11:
+      case Intl::HourCycle::kH12:
+        CHECK(JSReceiver::CreateDataProperty(isolate, options,
+                                             factory->hour12_string(),
+                                             factory->true_value(), kDontThrow)
+                  .FromJust());
+        break;
+      // iii. Else if, hc is "h23" or "h24", let v be false.
+      case Intl::HourCycle::kH23:
+      case Intl::HourCycle::kH24:
+        CHECK(JSReceiver::CreateDataProperty(isolate, options,
+                                             factory->hour12_string(),
+                                             factory->false_value(), kDontThrow)
+                  .FromJust());
+        break;
+      // iv. Else, let v be undefined.
+      case Intl::HourCycle::kUndefined:
+        break;
+    }
   }
 
-  const std::vector<PatternItem> items = GetPatternItems();
-  for (const auto& item : items) {
+  for (const auto& item : GetPatternItems()) {
     for (const auto& pair : item.pairs) {
       if (pattern.find(pair.pattern) != std::string::npos) {
-        // After we find the first pair in the item which matching the pattern,
-        // we set the property and look for the next item in kPatternItems.
         CHECK(JSReceiver::CreateDataProperty(
                   isolate, options,
                   factory->NewStringFromAsciiChecked(item.property.c_str()),
@@ -784,6 +794,21 @@ std::unique_ptr<icu::SimpleDateFormat> CreateICUDateFormat(
   return date_format;
 }
 
+Intl::HourCycle HourCycleDefault(icu::SimpleDateFormat* date_format) {
+  icu::UnicodeString pattern;
+  date_format->toPattern(pattern);
+  if (pattern.indexOf('K') >= 0) {
+    return Intl::HourCycle::kH11;
+  } else if (pattern.indexOf('h') >= 0) {
+    return Intl::HourCycle::kH12;
+  } else if (pattern.indexOf('H') >= 0) {
+    return Intl::HourCycle::kH23;
+  } else if (pattern.indexOf('k') >= 0) {
+    return Intl::HourCycle::kH24;
+  }
+  return Intl::HourCycle::kUndefined;
+}
+
 }  // namespace
 
 enum FormatMatcherOption { kBestFit, kBasic };
@@ -792,13 +817,13 @@ enum FormatMatcherOption { kBestFit, kBasic };
 MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::Initialize(
     Isolate* isolate, Handle<JSDateTimeFormat> date_time_format,
     Handle<Object> locales, Handle<Object> input_options) {
+  date_time_format->set_flags(0);
   // 1. Let requestedLocales be ? CanonicalizeLocaleList(locales).
   Maybe<std::vector<std::string>> maybe_requested_locales =
       Intl::CanonicalizeLocaleList(isolate, locales);
   MAYBE_RETURN(maybe_requested_locales, Handle<JSDateTimeFormat>());
   std::vector<std::string> requested_locales =
       maybe_requested_locales.FromJust();
-
   // 2. Let options be ? ToDateTimeOptions(options, "any", "date").
   Handle<JSObject> options;
   ASSIGN_RETURN_ON_EXCEPTION(
@@ -821,27 +846,20 @@ MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::Initialize(
   Maybe<bool> maybe_get_hour12 = Intl::GetBoolOption(
       isolate, options, "hour12", "Intl.DateTimeFormat", &hour12);
   MAYBE_RETURN(maybe_get_hour12, Handle<JSDateTimeFormat>());
-  HourOption hour_option = HourOption::H_UNKNOWN;
-  if (maybe_get_hour12.FromJust()) {
-    hour_option = hour12 ? HourOption::H_12 : HourOption::H_24;
-  }
 
   // 7. Let hourCycle be ? GetOption(options, "hourCycle", "string", « "h11",
   // "h12", "h23", "h24" », undefined).
   Maybe<Intl::HourCycle> maybe_hour_cycle =
       Intl::GetHourCycle(isolate, options, "Intl.DateTimeFormat");
   MAYBE_RETURN(maybe_hour_cycle, MaybeHandle<JSDateTimeFormat>());
-  // TODO(ftang): uncomment the following line and handle hour_cycle.
-  // Intl::HourCycle hour_cycle = maybe_hour_cycle.FromJust();
+  Intl::HourCycle hour_cycle = maybe_hour_cycle.FromJust();
 
   // 8. If hour12 is not undefined, then
   if (maybe_get_hour12.FromJust()) {
     // a. Let hourCycle be null.
-    // TODO(ftang): uncomment the following line and handle hour_cycle.
-    // hour_cycle = Intl::HourCycle::kUndefined;
+    hour_cycle = Intl::HourCycle::kUndefined;
   }
   // 9. Set opt.[[hc]] to hourCycle.
-  // TODO(ftang): change behavior based on hour_cycle.
 
   // ecma402/#sec-intl.datetimeformat-internal-slots
   // The value of the [[RelevantExtensionKeys]] internal slot is
@@ -860,6 +878,14 @@ MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::Initialize(
   icu::Locale icu_locale = r.icu_locale;
   DCHECK(!icu_locale.isBogus());
 
+  if (!maybe_get_hour12.FromJust() &&
+      hour_cycle == Intl::HourCycle::kUndefined) {
+    auto hc_extension_it = r.extensions.find("hc");
+    if (hc_extension_it != r.extensions.end()) {
+      hour_cycle = Intl::ToHourCycle(hc_extension_it->second.c_str());
+    }
+  }
+
   // 17. Let timeZone be ? Get(options, "timeZone").
   const std::vector<const char*> empty_values;
   std::unique_ptr<char[]> timezone = nullptr;
@@ -868,9 +894,36 @@ MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::Initialize(
                             "Intl.DateTimeFormat", &timezone);
   MAYBE_RETURN(maybe_timezone, Handle<JSDateTimeFormat>());
 
+  std::unique_ptr<icu::Calendar> calendar(
+      CreateCalendar(isolate, icu_locale, timezone.get()));
+
+  // 18.b If the result of IsValidTimeZoneName(timeZone) is false, then
+  // i. Throw a RangeError exception.
+  if (calendar.get() == nullptr) {
+    THROW_NEW_ERROR(isolate,
+                    NewRangeError(MessageTemplate::kInvalidTimeZone,
+                                  isolate->factory()->NewStringFromAsciiChecked(
+                                      timezone.get())),
+                    JSDateTimeFormat);
+  }
+
+  // 29.  If dateTimeFormat.[[Hour]] is not undefined, then
+  if (hour_cycle == Intl::HourCycle::kUndefined) {
+    // d. If hour12 is not undefined, then
+    if (maybe_get_hour12.FromJust()) {
+      // i. If hour12 is true, then
+      if (hour12) {
+        hour_cycle = Intl::HourCycle::kH12;
+      } else {  // ii. Else,
+        hour_cycle = Intl::HourCycle::kH23;
+      }
+    }
+  }
+
+  bool has_hour_option = false;
   // 22. For each row of Table 5, except the header row, do
   std::string skeleton;
-  for (const auto& item : GetPatternData(hour_option)) {
+  for (const PatternData& item : GetPatternData(hour_cycle)) {
     std::unique_ptr<char[]> input;
     // a. Let prop be the name given in the Property column of the row.
     // b. Let value be ? GetOption(options, prop, "string", « the strings given
@@ -880,6 +933,9 @@ MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::Initialize(
         "Intl.DateTimeFormat", &input);
     MAYBE_RETURN(maybe_get_option, Handle<JSDateTimeFormat>());
     if (maybe_get_option.FromJust()) {
+      if (item.property == "hour") {
+        has_hour_option = true;
+      }
       DCHECK_NOT_NULL(input.get());
       // c. Set opt.[[<prop>]] to value.
       skeleton += item.map.find(input.get())->second;
@@ -912,29 +968,33 @@ MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::Initialize(
     }
   }
 
-  // Set the locale
-  // 12. Set dateTimeFormat.[[Locale]] to r.[[locale]].
-  icu::Locale* cloned_locale = icu_locale.clone();
-  CHECK_NOT_NULL(cloned_locale);
-  Handle<Managed<icu::Locale>> managed_locale =
-      Managed<icu::Locale>::FromRawPtr(isolate, 0, cloned_locale);
-  date_time_format->set_icu_locale(*managed_locale);
-
+  // The creation of Calendar depends on timeZone so we have to put 13 after 17.
+  // Also date_format is not created until here.
   // 13. Set dateTimeFormat.[[Calendar]] to r.[[ca]].
-  std::unique_ptr<icu::Calendar> calendar(
-      CreateCalendar(isolate, icu_locale, timezone.get()));
+  date_format->adoptCalendar(calendar.release());
 
-  // 18.b If the result of IsValidTimeZoneName(timeZone) is false, then
-  // i. Throw a RangeError exception.
-  if (calendar.get() == nullptr) {
-    THROW_NEW_ERROR(isolate,
-                    NewRangeError(MessageTemplate::kInvalidTimeZone,
-                                  isolate->factory()->NewStringFromAsciiChecked(
-                                      timezone.get())),
-                    JSDateTimeFormat);
+  // 29. If dateTimeFormat.[[Hour]] is not undefined, then
+  if (has_hour_option) {
+    // a. Let hcDefault be dataLocaleData.[[hourCycle]].
+    Intl::HourCycle hc_default = HourCycleDefault(date_format.get());
+    // b. Let hc be dateTimeFormat.[[HourCycle]].
+    Intl::HourCycle hc = hour_cycle;
+    // c. If hc is null, then
+    if (hc == Intl::HourCycle::kUndefined) {
+      // i. Set hc to hcDefault.
+      hc = hc_default;
+    }
+    // e.  Set dateTimeFormat.[[HourCycle]] to hc.
+    date_time_format->set_hour_cycle(hc);
+    // 30. Else
+  } else {
+    // a. Set dateTimeFormat.[[HourCycle]] to undefined.
+    date_time_format->set_hour_cycle(Intl::HourCycle::kUndefined);
   }
-  date_format->adoptCalendar(calendar.release());
+  Handle<Managed<icu::Locale>> managed_locale =
+      Managed<icu::Locale>::FromRawPtr(isolate, 0, icu_locale.clone());
 
+  date_time_format->set_icu_locale(*managed_locale);
   Handle<Managed<icu::SimpleDateFormat>> managed_format =
       Managed<icu::SimpleDateFormat>::FromUniquePtr(isolate, 0,
                                                     std::move(date_format));
@@ -1061,5 +1121,22 @@ std::set<std::string> JSDateTimeFormat::GetAvailableLocales() {
   return Intl::BuildLocaleSet(icu_available_locales, num_locales);
 }
 
+Handle<String> JSDateTimeFormat::HourCycleAsString() const {
+  switch (hour_cycle()) {
+    case Intl::HourCycle::kUndefined:
+      return GetReadOnlyRoots().undefined_string_handle();
+    case Intl::HourCycle::kH11:
+      return GetReadOnlyRoots().h11_string_handle();
+    case Intl::HourCycle::kH12:
+      return GetReadOnlyRoots().h12_string_handle();
+    case Intl::HourCycle::kH23:
+      return GetReadOnlyRoots().h23_string_handle();
+    case Intl::HourCycle::kH24:
+      return GetReadOnlyRoots().h24_string_handle();
+    default:
+      UNREACHABLE();
+  }
+}
+
 }  // namespace internal
 }  // namespace v8

src/objects/js-date-time-format.h

@@ -73,6 +73,7 @@ class JSDateTimeFormat : public JSObject {
 
   static std::set<std::string> GetAvailableLocales();
 
+  Handle<String> HourCycleAsString() const;
   DECL_CAST2(JSDateTimeFormat)
 
 // Layout description.
@@ -80,6 +81,7 @@ class JSDateTimeFormat : public JSObject {
   V(kICULocaleOffset, kTaggedSize)           \
   V(kICUSimpleDateFormatOffset, kTaggedSize) \
   V(kBoundFormatOffset, kTaggedSize)         \
+  V(kFlagsOffset, kTaggedSize)               \
   /* Total size. */                          \
   V(kSize, 0)
 
@@ -87,9 +89,25 @@ class JSDateTimeFormat : public JSObject {
                                 JS_DATE_TIME_FORMAT_FIELDS)
 #undef JS_DATE_TIME_FORMAT_FIELDS
 
+  inline void set_hour_cycle(Intl::HourCycle hour_cycle);
+  inline Intl::HourCycle hour_cycle() const;
+
+// Bit positions in |flags|.
+#define FLAGS_BIT_FIELDS(V, _) V(HourCycleBits, Intl::HourCycle, 3, _)
+
+  DEFINE_BIT_FIELDS(FLAGS_BIT_FIELDS)
+#undef FLAGS_BIT_FIELDS
+
+  STATIC_ASSERT(Intl::HourCycle::kUndefined <= HourCycleBits::kMax);
+  STATIC_ASSERT(Intl::HourCycle::kH11 <= HourCycleBits::kMax);
+  STATIC_ASSERT(Intl::HourCycle::kH12 <= HourCycleBits::kMax);
+  STATIC_ASSERT(Intl::HourCycle::kH23 <= HourCycleBits::kMax);
+  STATIC_ASSERT(Intl::HourCycle::kH24 <= HourCycleBits::kMax);
+
   DECL_ACCESSORS(icu_locale, Managed<icu::Locale>)
   DECL_ACCESSORS(icu_simple_date_format, Managed<icu::SimpleDateFormat>)
   DECL_ACCESSORS(bound_format, Object)
+  DECL_INT_ACCESSORS(flags)
 
   DECL_PRINTER(JSDateTimeFormat)
   DECL_VERIFIER(JSDateTimeFormat)

test/test262/test262.status

@@ -677,7 +677,6 @@
   # to be either marked as bugs with issues filed for them or as deliberate
   # incompatibilities if the test cases turn out to be broken or ambiguous.
   # Some of these are related to v8:4361 in being visible side effects from Intl.
-  'intl402/DateTimeFormat/prototype/resolvedOptions/hourCycle': [FAIL],
 
   # https://bugs.chromium.org/p/v8/issues/detail?id=7833
   'built-ins/Atomics/wait/cannot-suspend-throws': [SKIP],