[Intl] Sync ListFormat to latest spec.

1. Add StringListFromIterable based on
https://tc39.es/proposal-intl-list-format/#sec-createstringlistfromiterable
2. Adjust other parts to sync with the new version.

Bug: v8:9747
Change-Id: I14202f2963463e6a3e9816209f087bfe8e73cb91
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1809902
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63899}

src/builtins/builtins-definitions.h

@@ -619,6 +619,9 @@ namespace internal {
   TFS(IterableToListWithSymbolLookup, kIterable)                               \
   TFS(IterableToListMayPreserveHoles, kIterable, kIteratorFn)                  \
                                                                                \
+  /* #sec-createstringlistfromiterable */                                      \
+  TFS(StringListFromIterable, kIterable)                                       \
+                                                                               \
   /* Map */                                                                    \
   TFS(FindOrderedHashMapEntry, kTable, kKey)                                   \
   TFJ(MapConstructor, SharedFunctionInfo::kDontAdaptArgumentsSentinel)         \

src/builtins/builtins-intl-gen.cc

@@ -162,28 +162,15 @@ void IntlBuiltinsAssembler::ListFormatCommon(TNode<Context> context,
                          method_name);
   TNode<JSListFormat> list_format = CAST(receiver);
 
-  // 4. If list is not provided or is undefined, then
   TNode<Object> list = args.GetOptionalArgumentValue(0);
-  Label has_list(this);
   {
-    GotoIfNot(IsUndefined(list), &has_list);
-    if (format_func_id == Runtime::kFormatList) {
-      // a. Return an empty String.
-      args.PopAndReturn(EmptyStringConstant());
-    } else {
-      DCHECK_EQ(format_func_id, Runtime::kFormatListToParts);
-      // a. Return an empty Array.
-      args.PopAndReturn(AllocateEmptyJSArray(context));
-    }
-  }
-  BIND(&has_list);
-  {
-    // 5. Let x be ? IterableToList(list).
-    TNode<Object> x =
-        CallBuiltin(Builtins::kIterableToListWithSymbolLookup, context, list);
+    // 4. Let stringList be ? StringListFromIterable(list).
+    TNode<Object> string_list =
+        CallBuiltin(Builtins::kStringListFromIterable, context, list);
 
-    // 6. Return ? FormatList(lf, x).
-    args.PopAndReturn(CallRuntime(format_func_id, context, list_format, x));
+    // 6. Return ? FormatList(lf, stringList).
+    args.PopAndReturn(
+        CallRuntime(format_func_id, context, list_format, string_list));
   }
 }
 

src/builtins/builtins-iterator-gen.cc

@@ -241,6 +241,69 @@ TF_BUILTIN(IterableToList, IteratorBuiltinsAssembler) {
   Return(IterableToList(context, iterable, iterator_fn));
 }
 
+TNode<JSArray> IteratorBuiltinsAssembler::StringListFromIterable(
+    TNode<Context> context, TNode<Object> iterable) {
+  Label done(this);
+  GrowableFixedArray list(state());
+  // 1. If iterable is undefined, then
+  //   a. Return a new empty List.
+  GotoIf(IsUndefined(iterable), &done);
+
+  // 2. Let iteratorRecord be ? GetIterator(items).
+  IteratorRecord iterator_record = GetIterator(context, iterable);
+
+  // 3. Let list be a new empty List.
+
+  Variable* vars[] = {list.var_array(), list.var_length(), list.var_capacity()};
+  Label loop_start(this, 3, vars);
+  Goto(&loop_start);
+  // 4. Let next be true.
+  // 5. Repeat, while next is not false
+  Label if_isnotstringtype(this, Label::kDeferred),
+      if_exception(this, Label::kDeferred);
+  BIND(&loop_start);
+  {
+    //  a. Set next to ? IteratorStep(iteratorRecord).
+    TNode<JSReceiver> next = IteratorStep(context, iterator_record, &done);
+    //  b. If next is not false, then
+    //   i. Let nextValue be ? IteratorValue(next).
+    TNode<Object> next_value = IteratorValue(context, next);
+    //   ii. If Type(nextValue) is not String, then
+    GotoIf(TaggedIsSmi(next_value), &if_isnotstringtype);
+    TNode<Uint16T> next_value_type = LoadInstanceType(CAST(next_value));
+    GotoIfNot(IsStringInstanceType(next_value_type), &if_isnotstringtype);
+    //   iii. Append nextValue to the end of the List list.
+    list.Push(next_value);
+    Goto(&loop_start);
+    // 5.b.ii
+    BIND(&if_isnotstringtype);
+    {
+      // 1. Let error be ThrowCompletion(a newly created TypeError object).
+      TVARIABLE(Object, var_exception);
+      TNode<Object> ret = CallRuntime(
+          Runtime::kThrowTypeError, context,
+          SmiConstant(MessageTemplate::kIterableYieldedNonString), next_value);
+      GotoIfException(ret, &if_exception, &var_exception);
+      Unreachable();
+
+      // 2. Return ? IteratorClose(iteratorRecord, error).
+      BIND(&if_exception);
+      IteratorCloseOnException(context, iterator_record, var_exception.value());
+    }
+  }
+
+  BIND(&done);
+  // 6. Return list.
+  return list.ToJSArray(context);
+}
+
+TF_BUILTIN(StringListFromIterable, IteratorBuiltinsAssembler) {
+  TNode<Context> context = CAST(Parameter(Descriptor::kContext));
+  TNode<Object> iterable = CAST(Parameter(Descriptor::kIterable));
+
+  Return(StringListFromIterable(context, iterable));
+}
+
 // This builtin always returns a new JSArray and is thus safe to use even in the
 // presence of code that may call back into user-JS. This builtin will take the
 // fast path if the iterable is a fast array and the Array prototype and the

src/builtins/builtins-iterator-gen.h

@@ -68,6 +68,11 @@ class IteratorBuiltinsAssembler : public CodeStubAssembler {
   TNode<JSArray> IterableToList(TNode<Context> context, TNode<Object> iterable,
                                 TNode<Object> iterator_fn);
 
+  // Currently at https://tc39.github.io/proposal-intl-list-format/
+  // #sec-createstringlistfromiterable
+  TNode<JSArray> StringListFromIterable(TNode<Context> context,
+                                        TNode<Object> iterable);
+
   void FastIterableToList(TNode<Context> context, TNode<Object> iterable,
                           TVariable<Object>* var_result, Label* slow);
 };

src/builtins/iterator.tq

@@ -37,6 +37,9 @@ namespace iterator {
   extern macro IteratorBuiltinsAssembler::IterableToList(
       implicit context: Context)(JSAny, JSAny): JSArray;
 
+  extern macro IteratorBuiltinsAssembler::StringListFromIterable(
+      implicit context: Context)(JSAny): JSArray;
+
   extern builtin IterableToListMayPreserveHoles(implicit context:
                                                     Context)(JSAny, JSAny);
   extern builtin IterableToListWithSymbolLookup(implicit context:

src/common/message-template.h

@@ -33,7 +33,6 @@ namespace internal {
     "Derived ArrayBuffer constructor created a buffer which was too small")    \
   T(ArrayBufferSpeciesThis,                                                    \
     "ArrayBuffer subclass returned this from species constructor")             \
-  T(ArrayItemNotType, "array %[%] is not type %")                              \
   T(AwaitNotInAsyncFunction, "await is only valid in async function")          \
   T(AtomicsWaitNotAllowed, "Atomics.wait cannot be called in this context")    \
   T(BadSortComparisonFunction,                                                 \
@@ -100,6 +99,7 @@ namespace internal {
   T(InvalidRegExpExecResult,                                                   \
     "RegExp exec method returned something other than an Object or null")      \
   T(InvalidUnit, "Invalid unit argument for %() '%'")                          \
+  T(IterableYieldedNonString, "Iterable yielded % which is not a string")      \
   T(IteratorResultNotAnObject, "Iterator result % is not an object")           \
   T(IteratorSymbolNonCallable, "Found non-callable @@iterator")                \
   T(IteratorValueNotAnObject, "Iterator value % is not an entry object")       \

src/objects/js-list-format.cc

@@ -252,35 +252,18 @@ namespace {
 // Extract String from JSArray into array of UnicodeString
 Maybe<std::vector<icu::UnicodeString>> ToUnicodeStringArray(
     Isolate* isolate, Handle<JSArray> array) {
-  Factory* factory = isolate->factory();
   // In general, ElementsAccessor::Get actually isn't guaranteed to give us the
   // elements in order. But if it is a holey array, it will cause the exception
   // with the IsString check.
   auto* accessor = array->GetElementsAccessor();
   uint32_t length = accessor->NumberOfElements(*array);
 
-  // ecma402 #sec-createpartsfromlist
-  // 2. If list contains any element value such that Type(value) is not String,
-  // throw a TypeError exception.
-  //
-  // Per spec it looks like we're supposed to throw a TypeError exception if the
-  // item isn't already a string, rather than coercing to a string.
   std::vector<icu::UnicodeString> result;
   for (uint32_t i = 0; i < length; i++) {
     DCHECK(accessor->HasElement(*array, i));
     Handle<Object> item = accessor->Get(array, i);
     DCHECK(!item.is_null());
-    if (!item->IsString()) {
-      THROW_NEW_ERROR_RETURN_VALUE(
-          isolate,
-          NewTypeError(MessageTemplate::kArrayItemNotType,
-                       factory->list_string(),
-                       // TODO(ftang): For dictionary-mode arrays, i isn't
-                       // actually the index in the array but the index in the
-                       // dictionary.
-                       factory->NewNumber(i), factory->String_string()),
-          Nothing<std::vector<icu::UnicodeString>>());
-    }
+    DCHECK(item->IsString());
     Handle<String> item_str = Handle<String>::cast(item);
     if (!item_str->IsFlat()) item_str = String::Flatten(isolate, item_str);
     result.push_back(Intl::ToICUUnicodeString(isolate, item_str));
@@ -294,9 +277,6 @@ MaybeHandle<T> FormatListCommon(
     Isolate* isolate, Handle<JSListFormat> format, Handle<JSArray> list,
     MaybeHandle<T> (*formatToResult)(Isolate*, const icu::FormattedValue&)) {
   DCHECK(!list->IsUndefined());
-  // ecma402 #sec-createpartsfromlist
-  // 2. If list contains any element value such that Type(value) is not String,
-  // throw a TypeError exception.
   Maybe<std::vector<icu::UnicodeString>> maybe_array =
       ToUnicodeStringArray(isolate, list);
   MAYBE_RETURN(maybe_array, Handle<T>());

test/intl/regress-9747.js

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let lf = new Intl.ListFormat("en");
+
+// Test normal array
+assertDoesNotThrow(() => lf.format(['a','b','c']));
+assertThrows("lf.format(['a','b',3])",  TypeError, "Iterable yielded 3 which is not a string");
+
+// Test sparse array
+let sparse = ['a','b'];
+sparse[10] = 'c';
+assertThrows("lf.format(sparse)",  TypeError, "Iterable yielded undefined which is not a string");
+
+// Test iterable of all String
+let iterable_of_strings = {
+  [Symbol.iterator]() {
+    return this;
+  },
+  count: 0,
+  next() {
+    if (this.count++ < 4) {
+      return {done: false, value: String(this.count)};
+    }
+    return {done:true}
+  }
+};
+assertDoesNotThrow(() => lf.format(iterable_of_strings));
+
+// Test iterable of none String throw TypeError
+let iterable_of_strings_and_number = {
+  [Symbol.iterator]() {
+    return this;
+  },
+  count: 0,
+  next() {
+    this.count++;
+    if (this.count ==  3) {
+      return {done:false, value: 3};
+    }
+    if (this.count < 5) {
+      return {done: false, value: String(this.count)};
+    }
+    return {done:true}
+  }
+};
+assertThrows("lf.format(iterable_of_strings_and_number)",
+    TypeError, "Iterable yielded 3 which is not a string");
+assertEquals(3, iterable_of_strings_and_number.count);