[Intl] Upgrade RelativeTimeFormat formatToParts

1. Add Intl::NumberFieldToType to support RelativeTimeFormat
by refactoring IcuNumberFieldIdToNumberType
2. Use formatNumericToValue / formatToValue to implement formatToParts

Bug: v8:8837
Change-Id: I4d8fab9c337ec02eeb3500b4c0f90547e48444e3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1560661Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Frank Tang <ftang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60797}

src/builtins/builtins-intl.cc

@@ -705,7 +705,7 @@ BUILTIN(RelativeTimeFormatPrototypeFormat) {
 
   RETURN_RESULT_OR_FAILURE(
       isolate, JSRelativeTimeFormat::Format(isolate, value_obj, unit_obj,
-                                            format_holder, "format", false));
+                                            format_holder));
 }
 
 BUILTIN(RelativeTimeFormatPrototypeFormatToParts) {
@@ -718,9 +718,9 @@ BUILTIN(RelativeTimeFormatPrototypeFormatToParts) {
                  "Intl.RelativeTimeFormat.prototype.formatToParts");
   Handle<Object> value_obj = args.atOrUndefined(isolate, 1);
   Handle<Object> unit_obj = args.atOrUndefined(isolate, 2);
-  RETURN_RESULT_OR_FAILURE(isolate, JSRelativeTimeFormat::Format(
-                                        isolate, value_obj, unit_obj,
-                                        format_holder, "formatToParts", true));
+  RETURN_RESULT_OR_FAILURE(
+      isolate, JSRelativeTimeFormat::FormatToParts(isolate, value_obj, unit_obj,
+                                                   format_holder));
 }
 
 // Locale getters.

src/objects/intl-objects.cc

@@ -1878,5 +1878,61 @@ const std::set<std::string>& Intl::GetAvailableLocalesForDateFormat() {
   return available_locales.Pointer()->Get();
 }
 
+Handle<String> Intl::NumberFieldToType(Isolate* isolate,
+                                       Handle<Object> numeric_obj,
+                                       int32_t field_id) {
+  DCHECK(numeric_obj->IsNumeric());
+  switch (static_cast<UNumberFormatFields>(field_id)) {
+    case UNUM_INTEGER_FIELD:
+      if (numeric_obj->IsBigInt()) {
+        // Neither NaN nor Infinite could be stored into BigInt
+        // so just return integer.
+        return isolate->factory()->integer_string();
+      } else {
+        double number = numeric_obj->Number();
+        if (std::isfinite(number)) return isolate->factory()->integer_string();
+        if (std::isnan(number)) return isolate->factory()->nan_string();
+        return isolate->factory()->infinity_string();
+      }
+    case UNUM_FRACTION_FIELD:
+      return isolate->factory()->fraction_string();
+    case UNUM_DECIMAL_SEPARATOR_FIELD:
+      return isolate->factory()->decimal_string();
+    case UNUM_GROUPING_SEPARATOR_FIELD:
+      return isolate->factory()->group_string();
+    case UNUM_CURRENCY_FIELD:
+      return isolate->factory()->currency_string();
+    case UNUM_PERCENT_FIELD:
+      return isolate->factory()->percentSign_string();
+    case UNUM_SIGN_FIELD:
+      if (numeric_obj->IsBigInt()) {
+        Handle<BigInt> big_int = Handle<BigInt>::cast(numeric_obj);
+        return big_int->IsNegative() ? isolate->factory()->minusSign_string()
+                                     : isolate->factory()->plusSign_string();
+      } else {
+        double number = numeric_obj->Number();
+        return number < 0 ? isolate->factory()->minusSign_string()
+                          : isolate->factory()->plusSign_string();
+      }
+    case UNUM_EXPONENT_SYMBOL_FIELD:
+    case UNUM_EXPONENT_SIGN_FIELD:
+    case UNUM_EXPONENT_FIELD:
+      // We should never get these because we're not using any scientific
+      // formatter.
+      UNREACHABLE();
+      return Handle<String>();
+
+    case UNUM_PERMILL_FIELD:
+      // We're not creating any permill formatter, and it's not even clear how
+      // that would be possible with the ICU API.
+      UNREACHABLE();
+      return Handle<String>();
+
+    default:
+      UNREACHABLE();
+      return Handle<String>();
+  }
+}
+
 }  // namespace internal
 }  // namespace v8

src/objects/intl-objects.h

@@ -20,7 +20,7 @@
 #include "unicode/locid.h"
 #include "unicode/uversion.h"
 
-#define V8_MINIMUM_ICU_VERSION 63
+#define V8_MINIMUM_ICU_VERSION 64
 
 namespace U_ICU_NAMESPACE {
 class BreakIterator;
@@ -186,6 +186,11 @@ class Intl {
       Isolate* isolate, const icu::UnicodeString& string, int32_t begin,
       int32_t end);
 
+  // Helper function to convert number field id to type string.
+  static Handle<String> NumberFieldToType(Isolate* isolate,
+                                          Handle<Object> numeric_obj,
+                                          int32_t field_id);
+
   // A helper function to implement formatToParts which add element to array as
   // $array[$index] = { type: $field_type_string, value: $value }
   static void AddElement(Isolate* isolate, Handle<JSArray> array, int index,

src/objects/js-number-format.cc

@@ -559,64 +559,6 @@ bool cmp_NumberFormatSpan(const NumberFormatSpan& a,
   return a.field_id < b.field_id;
 }
 
-// The list comes from third_party/icu/source/i18n/unicode/unum.h.
-// They're mapped to NumberFormat part types mentioned throughout
-// https://tc39.github.io/ecma402/#sec-partitionnumberpattern .
-Handle<String> IcuNumberFieldIdToNumberType(int32_t field_id,
-                                            Handle<Object> numeric_obj,
-                                            Isolate* isolate) {
-  DCHECK(numeric_obj->IsNumeric());
-  switch (static_cast<UNumberFormatFields>(field_id)) {
-    case UNUM_INTEGER_FIELD:
-      if (numeric_obj->IsBigInt()) {
-        // Neither NaN nor Infinite could be stored into BigInt
-        // so just return integer.
-        return isolate->factory()->integer_string();
-      } else {
-        double number = numeric_obj->Number();
-        if (std::isfinite(number)) return isolate->factory()->integer_string();
-        if (std::isnan(number)) return isolate->factory()->nan_string();
-        return isolate->factory()->infinity_string();
-      }
-    case UNUM_FRACTION_FIELD:
-      return isolate->factory()->fraction_string();
-    case UNUM_DECIMAL_SEPARATOR_FIELD:
-      return isolate->factory()->decimal_string();
-    case UNUM_GROUPING_SEPARATOR_FIELD:
-      return isolate->factory()->group_string();
-    case UNUM_CURRENCY_FIELD:
-      return isolate->factory()->currency_string();
-    case UNUM_PERCENT_FIELD:
-      return isolate->factory()->percentSign_string();
-    case UNUM_SIGN_FIELD:
-      if (numeric_obj->IsBigInt()) {
-        Handle<BigInt> big_int = Handle<BigInt>::cast(numeric_obj);
-        return big_int->IsNegative() ? isolate->factory()->minusSign_string()
-                                     : isolate->factory()->plusSign_string();
-      } else {
-        double number = numeric_obj->Number();
-        return number < 0 ? isolate->factory()->minusSign_string()
-                          : isolate->factory()->plusSign_string();
-      }
-    case UNUM_EXPONENT_SYMBOL_FIELD:
-    case UNUM_EXPONENT_SIGN_FIELD:
-    case UNUM_EXPONENT_FIELD:
-      // We should never get these because we're not using any scientific
-      // formatter.
-      UNREACHABLE();
-      return Handle<String>();
-
-    case UNUM_PERMILL_FIELD:
-      // We're not creating any permill formatter, and it's not even clear how
-      // that would be possible with the ICU API.
-      UNREACHABLE();
-      return Handle<String>();
-
-    default:
-      UNREACHABLE();
-      return Handle<String>();
-  }
-}
 }  // namespace
 
 // Flattens a list of possibly-overlapping "regions" to a list of
@@ -748,7 +690,7 @@ Maybe<int> JSNumberFormat::FormatToParts(Isolate* isolate,
     Handle<String> field_type_string =
         part.field_id == -1
             ? isolate->factory()->literal_string()
-            : IcuNumberFieldIdToNumberType(part.field_id, numeric_obj, isolate);
+            : Intl::NumberFieldToType(isolate, numeric_obj, part.field_id);
     Handle<String> substring;
     ASSIGN_RETURN_ON_EXCEPTION_VALUE(
         isolate, substring,

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

@@ -236,66 +236,6 @@ Handle<String> UnitAsString(Isolate* isolate, URelativeDateTimeUnit unit_enum) {
   }
 }
 
-MaybeHandle<JSArray> GenerateRelativeTimeFormatParts(
-    Isolate* isolate, const icu::UnicodeString& formatted,
-    const icu::UnicodeString& integer_part, URelativeDateTimeUnit unit_enum,
-    double number, const icu::NumberFormat& nf) {
-  Factory* factory = isolate->factory();
-  Handle<JSArray> array = factory->NewJSArray(0);
-  int32_t found = formatted.indexOf(integer_part);
-
-  Handle<String> substring;
-  if (found < 0) {
-    // Cannot find the integer_part in the formatted.
-    // Return [{'type': 'literal', 'value': formatted}]
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, substring,
-                               Intl::ToString(isolate, formatted), JSArray);
-    Intl::AddElement(isolate, array,
-                     0,                          // index
-                     factory->literal_string(),  // field_type_string
-                     substring);
-  } else {
-    // Found the formatted integer in the result.
-    int index = 0;
-
-    // array.push({
-    //     'type': 'literal',
-    //     'value': formatted.substring(0, found)})
-    if (found > 0) {
-      ASSIGN_RETURN_ON_EXCEPTION(isolate, substring,
-                                 Intl::ToString(isolate, formatted, 0, found),
-                                 JSArray);
-      Intl::AddElement(isolate, array, index++,
-                       factory->literal_string(),  // field_type_string
-                       substring);
-    }
-
-    Handle<String> unit = UnitAsString(isolate, unit_enum);
-
-    Handle<Object> number_obj = factory->NewNumber(number);
-    Maybe<int> maybe_format_to_parts = JSNumberFormat::FormatToParts(
-        isolate, array, index, nf, number_obj, unit);
-    MAYBE_RETURN(maybe_format_to_parts, Handle<JSArray>());
-    index = maybe_format_to_parts.FromJust();
-
-    // array.push({
-    //     'type': 'literal',
-    //     'value': formatted.substring(
-    //         found + integer_part.length, formatted.length)})
-    if (found + integer_part.length() < formatted.length()) {
-      ASSIGN_RETURN_ON_EXCEPTION(
-          isolate, substring,
-          Intl::ToString(isolate, formatted, found + integer_part.length(),
-                         formatted.length()),
-          JSArray);
-      Intl::AddElement(isolate, array, index,
-                       factory->literal_string(),  // field_type_string
-                       substring);
-    }
-  }
-  return array;
-}
-
 bool GetURelativeDateTimeUnit(Handle<String> unit,
                               URelativeDateTimeUnit* unit_enum) {
   std::unique_ptr<char[]> unit_str = unit->ToCString();
@@ -329,37 +269,32 @@ bool GetURelativeDateTimeUnit(Handle<String> unit,
   return true;
 }
 
-}  // namespace
-
-MaybeHandle<Object> JSRelativeTimeFormat::Format(
-    Isolate* isolate, Handle<Object> value_obj, Handle<Object> unit_obj,
-    Handle<JSRelativeTimeFormat> format_holder, const char* func_name,
-    bool to_parts) {
-  Factory* factory = isolate->factory();
-
+template <typename T>
+MaybeHandle<T> FormatCommon(
+    Isolate* isolate, Handle<JSRelativeTimeFormat> format,
+    Handle<Object> value_obj, Handle<Object> unit_obj, const char* func_name,
+    MaybeHandle<T> (*formatToResult)(Isolate*,
+                                     const icu::FormattedRelativeDateTime&,
+                                     Handle<Object>, Handle<String>)) {
   // 3. Let value be ? ToNumber(value).
   Handle<Object> value;
   ASSIGN_RETURN_ON_EXCEPTION(isolate, value,
-                             Object::ToNumber(isolate, value_obj), Object);
+                             Object::ToNumber(isolate, value_obj), T);
   double number = value->Number();
   // 4. Let unit be ? ToString(unit).
   Handle<String> unit;
   ASSIGN_RETURN_ON_EXCEPTION(isolate, unit, Object::ToString(isolate, unit_obj),
-                             Object);
-
+                             T);
   // 4. If isFinite(value) is false, then throw a RangeError exception.
   if (!std::isfinite(number)) {
     THROW_NEW_ERROR(
         isolate,
         NewRangeError(MessageTemplate::kNotFiniteNumber,
                       isolate->factory()->NewStringFromAsciiChecked(func_name)),
-        Object);
+        T);
   }
-
-  icu::RelativeDateTimeFormatter* formatter =
-      format_holder->icu_formatter()->raw();
+  icu::RelativeDateTimeFormatter* formatter = format->icu_formatter()->raw();
   CHECK_NOT_NULL(formatter);
-
   URelativeDateTimeUnit unit_enum;
   if (!GetURelativeDateTimeUnit(unit, &unit_enum)) {
     THROW_NEW_ERROR(
@@ -367,45 +302,137 @@ MaybeHandle<Object> JSRelativeTimeFormat::Format(
         NewRangeError(MessageTemplate::kInvalidUnit,
                       isolate->factory()->NewStringFromAsciiChecked(func_name),
                       unit),
-        Object);
+        T);
   }
-
   UErrorCode status = U_ZERO_ERROR;
-  icu::UnicodeString formatted;
-
-  if (format_holder->numeric() == JSRelativeTimeFormat::Numeric::ALWAYS) {
-    formatter->formatNumeric(number, unit_enum, formatted, status);
-  } else {
-    DCHECK_EQ(JSRelativeTimeFormat::Numeric::AUTO, format_holder->numeric());
-    formatter->format(number, unit_enum, formatted, status);
+  icu::FormattedRelativeDateTime formatted =
+      (format->numeric() == JSRelativeTimeFormat::Numeric::ALWAYS)
+          ? formatter->formatNumericToValue(number, unit_enum, status)
+          : formatter->formatToValue(number, unit_enum, status);
+  if (U_FAILURE(status)) {
+    THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kIcuError), T);
   }
+  return formatToResult(isolate, formatted, value,
+                        UnitAsString(isolate, unit_enum));
+}
 
+MaybeHandle<String> FormatToString(
+    Isolate* isolate, const icu::FormattedRelativeDateTime& formatted,
+    Handle<Object> value, Handle<String> unit) {
+  UErrorCode status = U_ZERO_ERROR;
+  icu::UnicodeString result = formatted.toString(status);
   if (U_FAILURE(status)) {
-    THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kIcuError), Object);
+    THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kIcuError), String);
   }
+  return Intl::ToString(isolate, result);
+}
 
-  if (to_parts) {
-    icu::UnicodeString number_str;
-    icu::FieldPosition pos;
-    double abs_number = std::abs(number);
-    formatter->getNumberFormat().format(abs_number, number_str, pos, status);
+Maybe<bool> AddLiteral(Isolate* isolate, Handle<JSArray> array,
+                       const icu::UnicodeString& string, int32_t index,
+                       int32_t start, int32_t limit) {
+  Handle<String> substring;
+  ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+      isolate, substring, Intl::ToString(isolate, string, start, limit),
+      Nothing<bool>());
+  Intl::AddElement(isolate, array, index, isolate->factory()->literal_string(),
+                   substring);
+  return Just(true);
+}
+
+Maybe<bool> AddUnit(Isolate* isolate, Handle<JSArray> array,
+                    const icu::UnicodeString& string, int32_t index,
+                    int32_t start, int32_t limit, int32_t field_id,
+                    Handle<Object> value, Handle<String> unit) {
+  Handle<String> substring;
+  ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+      isolate, substring, Intl::ToString(isolate, string, start, limit),
+      Nothing<bool>());
+  Intl::AddElement(isolate, array, index,
+                   Intl::NumberFieldToType(isolate, value, field_id), substring,
+                   isolate->factory()->unit_string(), unit);
+  return Just(true);
+}
+
+MaybeHandle<JSArray> FormatToJSArray(
+    Isolate* isolate, const icu::FormattedRelativeDateTime& formatted,
+    Handle<Object> value, Handle<String> unit) {
+  UErrorCode status = U_ZERO_ERROR;
+  icu::UnicodeString string = formatted.toString(status);
+
+  Factory* factory = isolate->factory();
+  Handle<JSArray> array = factory->NewJSArray(0);
+  icu::ConstrainedFieldPosition cfpos;
+  cfpos.constrainCategory(UFIELD_CATEGORY_NUMBER);
+  int32_t index = 0;
+
+  int32_t previous_end = 0;
+  Handle<String> substring;
+  std::vector<std::pair<int32_t, int32_t>> groups;
+  while (formatted.nextPosition(cfpos, status) && U_SUCCESS(status)) {
+    int32_t category = cfpos.getCategory();
+    int32_t field = cfpos.getField();
+    int32_t start = cfpos.getStart();
+    int32_t limit = cfpos.getLimit();
+    if (category == UFIELD_CATEGORY_NUMBER) {
+      if (field == UNUM_GROUPING_SEPARATOR_FIELD) {
+        groups.push_back(std::pair<int32_t, int32_t>(start, limit));
+        continue;
+      }
+      if (start > previous_end) {
+        Maybe<bool> maybe_added =
+            AddLiteral(isolate, array, string, index++, previous_end, start);
+        MAYBE_RETURN(maybe_added, Handle<JSArray>());
+      }
+      if (field == UNUM_INTEGER_FIELD) {
+        for (auto start_limit : groups) {
+          if (start_limit.first > start) {
+            Maybe<bool> maybe_added =
+                AddUnit(isolate, array, string, index++, start,
+                        start_limit.first, field, value, unit);
+            MAYBE_RETURN(maybe_added, Handle<JSArray>());
+            maybe_added = AddUnit(isolate, array, string, index++,
+                                  start_limit.first, start_limit.second,
+                                  UNUM_GROUPING_SEPARATOR_FIELD, value, unit);
+            MAYBE_RETURN(maybe_added, Handle<JSArray>());
+            start = start_limit.second;
+          }
+        }
+      }
+      Maybe<bool> maybe_added = AddUnit(isolate, array, string, index++, start,
+                                        limit, field, value, unit);
+      MAYBE_RETURN(maybe_added, Handle<JSArray>());
+      previous_end = limit;
+    }
+  }
   if (U_FAILURE(status)) {
-      THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kIcuError),
-                      Object);
+    THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kIcuError), JSArray);
   }
-
-    Handle<JSArray> elements;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, elements,
-                               GenerateRelativeTimeFormatParts(
-                                   isolate, formatted, number_str, unit_enum,
-                                   abs_number, formatter->getNumberFormat()),
-                               Object);
-    return elements;
+  if (string.length() > previous_end) {
+    Maybe<bool> maybe_added = AddLiteral(isolate, array, string, index,
+                                         previous_end, string.length());
+    MAYBE_RETURN(maybe_added, Handle<JSArray>());
   }
 
-  return factory->NewStringFromTwoByte(Vector<const uint16_t>(
-      reinterpret_cast<const uint16_t*>(formatted.getBuffer()),
-      formatted.length()));
+  JSObject::ValidateElements(*array);
+  return array;
+}
+
+}  // namespace
+
+MaybeHandle<String> JSRelativeTimeFormat::Format(
+    Isolate* isolate, Handle<Object> value_obj, Handle<Object> unit_obj,
+    Handle<JSRelativeTimeFormat> format) {
+  return FormatCommon<String>(isolate, format, value_obj, unit_obj,
+                              "Intl.RelativeTimeFormat.prototype.format",
+                              FormatToString);
+}
+
+MaybeHandle<JSArray> JSRelativeTimeFormat::FormatToParts(
+    Isolate* isolate, Handle<Object> value_obj, Handle<Object> unit_obj,
+    Handle<JSRelativeTimeFormat> format) {
+  return FormatCommon<JSArray>(
+      isolate, format, value_obj, unit_obj,
+      "Intl.RelativeTimeFormat.prototype.formatToParts", FormatToJSArray);
 }
 
 const std::set<std::string>& JSRelativeTimeFormat::GetAvailableLocales() {

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

@@ -44,11 +44,14 @@ class JSRelativeTimeFormat : public JSObject {
   Handle<String> NumericAsString() const;
 
   // ecma402/#sec-Intl.RelativeTimeFormat.prototype.format
+  V8_WARN_UNUSED_RESULT static MaybeHandle<String> Format(
+      Isolate* isolate, Handle<Object> value_obj, Handle<Object> unit_obj,
+      Handle<JSRelativeTimeFormat> format);
+
   // ecma402/#sec-Intl.RelativeTimeFormat.prototype.formatToParts
-  V8_WARN_UNUSED_RESULT static MaybeHandle<Object> Format(
+  V8_WARN_UNUSED_RESULT static MaybeHandle<JSArray> FormatToParts(
       Isolate* isolate, Handle<Object> value_obj, Handle<Object> unit_obj,
-      Handle<JSRelativeTimeFormat> format_holder, const char* func_name,
-      bool to_parts);
+      Handle<JSRelativeTimeFormat> format);
 
   V8_EXPORT_PRIVATE static const std::set<std::string>& GetAvailableLocales();
 
@@ -107,7 +110,6 @@ class JSRelativeTimeFormat : public JSObject {
 
   // Layout description.
 #define JS_RELATIVE_TIME_FORMAT_FIELDS(V)     \
-  V(kJSRelativeTimeFormatOffset, kTaggedSize) \
   V(kLocaleOffset, kTaggedSize)               \
   V(kICUFormatterOffset, kTaggedSize)         \
   V(kFlagsOffset, kTaggedSize)                \