[builtins] move String.prototype[@@iterator] to C++ builtin

src/builtins/builtins-string.cc

Index: src/builtins/builtins-string.cc
diff --git a/src/builtins/builtins-string.cc b/src/builtins/builtins-string.cc
index f7413fc30ff6681293e0c8157c4fdc7b5f03e0c8..791c52060d1f74d810cf24042e957d1e715e5f9e 100644
--- a/src/builtins/builtins-string.cc
+++ b/src/builtins/builtins-string.cc
@@ -607,5 +607,63 @@ void Builtins::Generate_StringPrototypeValueOf(CodeStubAssembler* assembler) {
   assembler->Return(result);
 }
 
+BUILTIN(StringPrototypeIterator) {
+  HandleScope scope(isolate);
+  TO_THIS_STRING(object, "String.prototype[Symbol.iterator]");
+
+  Handle<String> string;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, string,
+                                     Object::ToString(isolate, object));
+
+  return *isolate->factory()->NewJSStringIterator(string);
+}
+
+BUILTIN(StringIteratorPrototypeNext) {
+  HandleScope scope(isolate);
+
+  if (!args.receiver()->IsJSStringIterator()) {
+    Handle<String> reason = isolate->factory()->NewStringFromAsciiChecked(
+        "String Iterator.prototype.next");
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate,
+        NewTypeError(MessageTemplate::kIncompatibleMethodReceiver, reason));
+  }
+  Handle<JSStringIterator> iterator =
+      Handle<JSStringIterator>::cast(args.receiver());
+  Handle<String> string(iterator->string());
+
+  if (*string != isolate->heap()->empty_string()) {

[Benedikt Meurer, 2016/09/19 04:05:18]

You don't need this check, as the empty string will fail the position < length
below. That's the idea behind using the empty string instead of undefined.

[caitp, 2016/09/19 16:03:04]

Done.

+    int position = iterator->index();
+    int length = string->length();
+
+    do {
+      if (position >= length) break;
+
+      uint16_t lead = string->Get(position);
+      if (lead >= 0xD800 && lead <= 0xDBFF && position + 1 < length) {
+        uint16_t trail = string->Get(position + 1);
+        if (V8_LIKELY(trail >= 0xDC00 && trail <= 0xDFFF)) {
+          // Return combined code units
+          iterator->set_index(position + 2);
+          Handle<String> value = isolate->factory()->NewProperSubString(

[Benedikt Meurer, 2016/09/19 04:05:18]

I'd prefer to not use NewProperSubString here, as we know anyways that we have
to create a two byte string with 2 characters for the surrogate pair. Maybe we
can have a simple NewSurrogatePairString function here?

[caitp, 2016/09/19 16:03:05]

Done --- Should the new factory method try to lookup a match in the interned
string table? I thought it might not be worth it to spend time doing the hash
lookup for (probably) short lived small strings.

[Benedikt Meurer, 2016/09/20 04:57:53]

Nope, that doesn't make sense. Especially since the stub/inlined code should
just create a two char string and be done with it.

[caitp, 2016/09/20 13:28:14]

Acknowledged.

+              string, position, position + 2);
+          return *isolate->factory()->NewJSIteratorResult(value, false);
+        }
+      }
+
+      // Return single code unit
+      iterator->set_index(position + 1);
+      Handle<String> value = isolate->factory()->NewProperSubString(
+          string, position, position + 1);

[Benedikt Meurer, 2016/09/19 04:05:18]

Please use LookupSingleCharacterStringFromCode directly here.

[caitp, 2016/09/19 16:03:04]

Done.

+      return *isolate->factory()->NewJSIteratorResult(value, false);
+    } while (false);
+
+    iterator->set_string(isolate->heap()->empty_string());
+  }
+
+  return *isolate->factory()->NewJSIteratorResult(
+      isolate->factory()->undefined_value(), true);
+}
+
 }  // namespace internal
 }  // namespace v8