for-of should throw if result object is not an object

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 54e4c7cec15b1ed18f2fedcf36db47fb8ed6a126..84794d10a9a1cc039bf595a51384b0ef524cbd80 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -2925,25 +2925,50 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt,
     Expression* result_done;
     Expression* assign_each;
 
-    // var iterator = subject[Symbol.iterator]();
+    // iterator = subject[Symbol.iterator]()
     assign_iterator = factory()->NewAssignment(
         Token::ASSIGN, factory()->NewVariableProxy(iterator),
         GetIterator(subject, factory()), subject->position());
 
-    // var result = iterator.next();
+    // !%_IsSpecObject(result = iterator.next()) &&
+    //     %ThrowIteratorResultNotAnObject(result)
     {
+      // result = iterator.next()
       Expression* iterator_proxy = factory()->NewVariableProxy(iterator);
       Expression* next_literal = factory()->NewStringLiteral(
           ast_value_factory()->next_string(), RelocInfo::kNoPosition);
       Expression* next_property = factory()->NewProperty(
           iterator_proxy, next_literal, RelocInfo::kNoPosition);
       ZoneList<Expression*>* next_arguments =
-          new(zone()) ZoneList<Expression*>(0, zone());
+          new (zone()) ZoneList<Expression*>(0, zone());
       Expression* next_call = factory()->NewCall(next_property, next_arguments,
                                                  subject->position());
       Expression* result_proxy = factory()->NewVariableProxy(result);
       next_result = factory()->NewAssignment(Token::ASSIGN, result_proxy,
                                              next_call, subject->position());
+
+      // %_IsSpecObject(...)
+      ZoneList<Expression*>* is_spec_object_args =
+          new (zone()) ZoneList<Expression*>(1, zone());
+      is_spec_object_args->Add(next_result, zone());
+      Expression* is_spec_object_call = factory()->NewCallRuntime(
+          NULL, Runtime::FunctionForId(Runtime::kInlineIsSpecObject),

[adamk, 2015/02/23 22:17:10]

I think you need to pass a non-NULL AstRawString here, otherwise Crankshaft will
crash in a DCHECK:

https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/hydrogen.cc...

Looks like we already do this for %_IsConstructCall() (is_construct_call_string
is in ast-value-factory.h).

+          is_spec_object_args, subject->position());
+
+      // %ThrowIteratorResultNotAnObject(result)
+      Expression* result_proxy_again = factory()->NewVariableProxy(result);
+      ZoneList<Expression*>* throw_arguments =
+          new (zone()) ZoneList<Expression*>(1, zone());
+      throw_arguments->Add(result_proxy_again, zone());
+      Expression* throw_call = factory()->NewCallRuntime(
+          NULL,

[adamk, 2015/02/23 22:17:10]

Ditto, you should pass a real name here.

[arv (Not doing code reviews), 2015/02/23 22:26:09]

I added the strings but I ran this in debug mode without hitting the assert.

Probably because Crankshaft does not support for-of.

[adamk, 2015/02/23 22:35:35]

Hmm, ok, looks like empty_string is used in some other places, so I guess this
is fine. I'm not sure I see the downside to giving these things proper names,
though.

[arv (Not doing code reviews), 2015/02/23 22:51:54]

I added the strings.

+          Runtime::FunctionForId(Runtime::kThrowIteratorResultNotAnObject),
+          throw_arguments, subject->position());
+
+      next_result = factory()->NewBinaryOperation(
+          Token::AND, factory()->NewUnaryOperation(
+                          Token::NOT, is_spec_object_call, subject->position()),
+          throw_call, subject->position());
     }
 
     // result.done