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

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

This is done using desugaring. Before this we had:

  result = iterator.next()

with this we instead do:

  !%_IsSpecObject(result = iterator.next()) &&
      %ThrowIteratorResultNotAnObject(result)

BUG=v8:3916
LOG=N

Committed: https://crrev.com/ab2591ed9332086581ee25ef735cd1b3bbd7e2f0
Cr-Commit-Position: refs/heads/master@{#26806}

[arv (Not doing code reviews), 2015-02-23 22:01:59]

Fix some comments

[arv (Not doing code reviews), 2015-02-23 22:02:23]

arv@chromium.org changed reviewers:
+ adamk@chromium.org, wingo@igalia.com

[arv (Not doing code reviews), 2015-02-23 22:02:24]

https://codereview.chromium.org/929733003/diff/1/src/runtime/runtime-generato...
File src/runtime/runtime-generator.cc (right):

https://codereview.chromium.org/929733003/diff/1/src/runtime/runtime-generato...
src/runtime/runtime-generator.cc:228:
RUNTIME_FUNCTION(Runtime_ThrowIteratorResultNotAnObject) {
I wasn't sure if this was the best place to put this.

https://codereview.chromium.org/929733003/diff/1/src/runtime/runtime.h
File src/runtime/runtime.h (right):

https://codereview.chromium.org/929733003/diff/1/src/runtime/runtime.h#newcode92
src/runtime/runtime.h:92: F(ThrowIteratorResultNotAnObject, 1, 1)      \
Sigh, git cl format

https://codereview.chromium.org/929733003/diff/1/test/mjsunit/es6/iteration-s...
File test/mjsunit/es6/iteration-semantics.js (right):

https://codereview.chromium.org/929733003/diff/1/test/mjsunit/es6/iteration-s...
test/mjsunit/es6/iteration-semantics.js:203: assertThrows(function() {
More extensive tests below

[adamk, 2015-02-23 22:17:11]

https://codereview.chromium.org/929733003/diff/20001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/929733003/diff/20001/src/parser.cc#newcode2955
src/parser.cc:2955: NULL, Runtime::FunctionForId(Runtime::kInlineIsSpecObject),
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).

https://codereview.chromium.org/929733003/diff/20001/src/parser.cc#newcode2964
src/parser.cc:2964: NULL,
Ditto, you should pass a real name here.

https://codereview.chromium.org/929733003/diff/20001/src/runtime/runtime-gene...
File src/runtime/runtime-generator.cc (right):

https://codereview.chromium.org/929733003/diff/20001/src/runtime/runtime-gene...
src/runtime/runtime-generator.cc:228:
RUNTIME_FUNCTION(Runtime_ThrowIteratorResultNotAnObject) {
Can you put this up before the two RuntimeReference functions?

[arv (Not doing code reviews), 2015-02-23 22:24:40]

Use empty string instead of null

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

PTAL

https://codereview.chromium.org/929733003/diff/20001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/929733003/diff/20001/src/parser.cc#newcode2964
src/parser.cc:2964: NULL,
On 2015/02/23 22:17:10, adamk wrote:
> Ditto, you should pass a real name here.

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

Probably because Crankshaft does not support for-of.

https://codereview.chromium.org/929733003/diff/20001/src/runtime/runtime-gene...
File src/runtime/runtime-generator.cc (right):

https://codereview.chromium.org/929733003/diff/20001/src/runtime/runtime-gene...
src/runtime/runtime-generator.cc:228:
RUNTIME_FUNCTION(Runtime_ThrowIteratorResultNotAnObject) {
On 2015/02/23 22:17:11, adamk wrote:
> Can you put this up before the two RuntimeReference functions?

That is clearly better.

My q was  more about which file. for-of is not tied to generators so maybe this
should go somewhere else?

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

lgtm

https://codereview.chromium.org/929733003/diff/20001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/929733003/diff/20001/src/parser.cc#newcode2964
src/parser.cc:2964: NULL,
On 2015/02/23 22:26:09, arv wrote:
> On 2015/02/23 22:17:10, adamk wrote:
> > Ditto, you should pass a real name here.
> 
> I added the strings but I ran this in debug mode without hitting the assert.
> 
> Probably because Crankshaft does not support for-of.

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.

https://codereview.chromium.org/929733003/diff/20001/src/runtime/runtime-gene...
File src/runtime/runtime-generator.cc (right):

https://codereview.chromium.org/929733003/diff/20001/src/runtime/runtime-gene...
src/runtime/runtime-generator.cc:228:
RUNTIME_FUNCTION(Runtime_ThrowIteratorResultNotAnObject) {
On 2015/02/23 22:26:09, arv wrote:
> On 2015/02/23 22:17:11, adamk wrote:
> > Can you put this up before the two RuntimeReference functions?
> 
> That is clearly better.
> 
> My q was  more about which file. for-of is not tied to generators so maybe
this
> should go somewhere else?

I think the set of runtime-foo.cc files is pretty arbitrary. You could just
create a new runtime-iterators.cc file.

[arv (Not doing code reviews), 2015-02-23 22:50:47]

add strings and move runtime function

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

The CQ bit was checked by arv@chromium.org

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

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/929733003/#ps60001 (title:
"add strings and move runtime function")

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

Thanks

https://codereview.chromium.org/929733003/diff/20001/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/929733003/diff/20001/src/parser.cc#newcode2964
src/parser.cc:2964: NULL,
On 2015/02/23 22:35:35, adamk wrote:
> On 2015/02/23 22:26:09, arv wrote:
> > On 2015/02/23 22:17:10, adamk wrote:
> > > Ditto, you should pass a real name here.
> > 
> > I added the strings but I ran this in debug mode without hitting the assert.
> > 
> > Probably because Crankshaft does not support for-of.
> 
> 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.

I added the strings.

https://codereview.chromium.org/929733003/diff/20001/src/runtime/runtime-gene...
File src/runtime/runtime-generator.cc (right):

https://codereview.chromium.org/929733003/diff/20001/src/runtime/runtime-gene...
src/runtime/runtime-generator.cc:228:
RUNTIME_FUNCTION(Runtime_ThrowIteratorResultNotAnObject) {
On 2015/02/23 22:35:35, adamk wrote:
> On 2015/02/23 22:26:09, arv wrote:
> > On 2015/02/23 22:17:11, adamk wrote:
> > > Can you put this up before the two RuntimeReference functions?
> > 
> > That is clearly better.
> > 
> > My q was  more about which file. for-of is not tied to generators so maybe
> this
> > should go somewhere else?
> 
> I think the set of runtime-foo.cc files is pretty arbitrary. You could just
> create a new runtime-iterators.cc file.

Moved to runtime-internal.cc which has ThrowReferenceError.

[commit-bot: I haz the power, 2015-02-23 22:52:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/929733003/60001

[commit-bot: I haz the power, 2015-02-23 23:34:33]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-23 23:34:39]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-23 23:35:02]

Patchset 4 (id:??) landed as
https://crrev.com/87f77d6abaf8746719ae1f53b9eb61c1d1cf6a0b
Cr-Commit-Position: refs/heads/master@{#26805}

[commit-bot: I haz the power, 2015-02-23 23:35:05]

Patchset 4 (id:??) landed as
https://crrev.com/ab2591ed9332086581ee25ef735cd1b3bbd7e2f0
Cr-Commit-Position: refs/heads/master@{#26806}

[arv (Not doing code reviews), 2015-02-23 23:52:02]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/946943003/ by arv@chromium.org.

The reason for reverting is: Broke compile.