[es6] ship @@toStringTag

[es6] ship @@toStringTag

BUG=v8:3502
R=arv@chromium.org
LOG=N

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

[caitp (gmail), 2015-04-08 18:01:44]

PTAL

[arv (Not doing code reviews), 2015-04-08 18:04:15]

LGTM

[caitp (gmail), 2015-04-08 18:08:12]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2015-04-08 18:08:16]

The CQ bit was unchecked by caitpotter88@gmail.com

[caitp (gmail), 2015-04-08 18:08:27]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-04-08 18:08:48]

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

[commit-bot: I haz the power, 2015-04-08 18:25:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-08 18:25:11]

Try jobs failed on following builders:
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[caitp (gmail), 2015-04-08 18:36:15]

On 2015/04/08 18:25:11, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

I think this is one of those "strict mode natives" issues :( should the test
just be deleted?

[caitp (gmail), 2015-04-08 18:37:23]

On 2015/04/08 18:36:15, caitp wrote:
> On 2015/04/08 18:25:11, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)
> 
> I think this is one of those "strict mode natives" issues :( should the test
> just be deleted?

There's no web compat issue, (or at least, FF throws trying to access it), so I
think it's safe to get rid of it

[caitp (gmail), 2015-04-08 18:57:41]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2015-04-08 18:57:41]

The patchset sent to the CQ was uploaded after l-g-t-m from arv@chromium.org
Link to the patchset: https://codereview.chromium.org/1072573002/#ps20001
(title: "Fix function-arguments-null test")

[commit-bot: I haz the power, 2015-04-08 18:58:04]

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

[arv (Not doing code reviews), 2015-04-08 19:01:38]

LGTM

That test is much improved over the old one.

[commit-bot: I haz the power, 2015-04-08 19:23:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-08 19:23:22]

Try jobs failed on following builders:
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/4560)

[caitp (gmail), 2015-04-08 19:36:20]

On 2015/04/08 19:23:22, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/4560)

that one looks unrelated to me. flake?

[arv (Not doing code reviews), 2015-04-08 20:14:17]

On 2015/04/08 19:36:20, caitp wrote:
> On 2015/04/08 19:23:22, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/4560)
> 
> that one looks unrelated to me. flake?

You can try again.

[caitp (gmail), 2015-04-08 20:24:40]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-04-08 20:25:11]

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

[commit-bot: I haz the power, 2015-04-08 21:00:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-08 21:00:27]

Try jobs failed on following builders:
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/4564)

[caitp (gmail), 2015-04-09 01:06:55]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-04-09 01:07:30]

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

[commit-bot: I haz the power, 2015-04-09 01:34:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-09 01:34:18]

Try jobs failed on following builders:
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/4568)

[caitp (gmail), 2015-04-09 01:35:46]

On 2015/04/09 01:34:18, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/4568)

jeeze, okay, will deal with that tomorrow ._> this is turning out to be sort of
hard to ship =]

[caitp (gmail), 2015-04-09 02:16:50]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2015-04-09 02:16:50]

The patchset sent to the CQ was uploaded after l-g-t-m from arv@chromium.org
Link to the patchset: https://codereview.chromium.org/1072573002/#ps40001
(title: "Fix prototype-ordinary-objects test")

[commit-bot: I haz the power, 2015-04-09 02:17:19]

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

[caitp (gmail), 2015-04-09 02:17:32]

The CQ bit was unchecked by caitpotter88@gmail.com

[arv (Not doing code reviews), 2015-04-09 13:41:22]

LGTM

https://codereview.chromium.org/1072573002/diff/40001/test/mjsunit/es6/protot...
File test/mjsunit/es6/prototype-ordinary-objects.js (right):

https://codereview.chromium.org/1072573002/diff/40001/test/mjsunit/es6/protot...
test/mjsunit/es6/prototype-ordinary-objects.js:59: // %TypedArray% [
@@toStringTag ] is an accessor which returns undefined if
Do we have a bug then? Float32Array etc above should have a @@toStringTag.

Maybe the above test should first have deleted the @@toStringTag?

https://codereview.chromium.org/1072573002/diff/40001/test/mjsunit/es6/protot...
test/mjsunit/es6/prototype-ordinary-objects.js:76: assertEquals(`[object
${name}]`, Object.prototype.toString.call(p));
update line 54 for consistency?

[caitp (gmail), 2015-04-09 14:10:23]

https://codereview.chromium.org/1072573002/diff/40001/test/mjsunit/es6/protot...
File test/mjsunit/es6/prototype-ordinary-objects.js (right):

https://codereview.chromium.org/1072573002/diff/40001/test/mjsunit/es6/protot...
test/mjsunit/es6/prototype-ordinary-objects.js:59: // %TypedArray% [
@@toStringTag ] is an accessor which returns undefined if
On 2015/04/09 13:41:22, arv wrote:
> Do we have a bug then? Float32Array etc above should have a @@toStringTag.

They have @@toStringTag --- it's a getter which is spec'd to return undefined if
`this` is not a %TypedArray% (eg. when getting it via the prototype, which is an
ordinary object).

It's doing the right thing here

> Maybe the above test should first have deleted the @@toStringTag?

Hmm --- since @@toStringTag is tested separately, I guess it's simpler to just
do that

https://codereview.chromium.org/1072573002/diff/40001/test/mjsunit/es6/protot...
test/mjsunit/es6/prototype-ordinary-objects.js:76: assertEquals(`[object
${name}]`, Object.prototype.toString.call(p));
On 2015/04/09 13:41:22, arv wrote:
> update line 54 for consistency?

Acknowledged.

[caitp (gmail), 2015-04-09 14:18:16]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2015-04-09 14:18:16]

The patchset sent to the CQ was uploaded after l-g-t-m from arv@chromium.org
Link to the patchset: https://codereview.chromium.org/1072573002/#ps60001
(title: "Simpler fixfor prototype-ordinary-objects.js")

[commit-bot: I haz the power, 2015-04-09 14:18:28]

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

[arv (Not doing code reviews), 2015-04-09 14:30:52]

Still LGTM

[commit-bot: I haz the power, 2015-04-09 14:46:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-09 14:46:24]

Try jobs failed on following builders:
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/4599)

[caitp (gmail), 2015-04-09 15:25:23]

On 2015/04/09 14:46:24, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/4599)

It seems that old code-space is larger on arm64 with this flag enabled, because
Object.prototype.toString is replaced, and the arithmetic just works out that
way with this replacement on ARM.

So, a bit more work, but ObjectToString needs to be not installed if this flag
is flipped on.

[caitp (gmail), 2015-04-09 15:50:26]

On 2015/04/09 15:25:23, caitp wrote:
> On 2015/04/09 14:46:24, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/4599)
> 
> It seems that old code-space is larger on arm64 with this flag enabled,
because
> Object.prototype.toString is replaced, and the arithmetic just works out that
> way with this replacement on ARM.
> 
> So, a bit more work, but ObjectToString needs to be not installed if this flag
> is flipped on.

I've updated the code so that it doesn't "replace" an old property (makes
harmony-tostring.js a bit smaller, and v8natives.js/runtime-internal.cc slightly
bigger), but it fixes the test failure...

Sigh, it's getting a bit big for a "flip the switch" patch :| think I prefer to
move these extra changes into a separate CL which can land first, so that it's
easier to revert if necessary

[rossberg, 2015-04-09 16:11:54]

On 2015/04/09 15:50:26, caitp wrote:
> Sigh, it's getting a bit big for a "flip the switch" patch :| think I prefer
to
> move these extra changes into a separate CL which can land first, so that it's
> easier to revert if necessary

Yes, sounds like a good idea.

[caitp (gmail), 2015-04-09 22:46:33]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2015-04-09 22:46:34]

The patchset sent to the CQ was uploaded after l-g-t-m from arv@chromium.org
Link to the patchset: https://codereview.chromium.org/1072573002/#ps120001
(title: "Do test-fixing in crrev.com/1072083002")

[commit-bot: I haz the power, 2015-04-09 22:46:53]

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

[commit-bot: I haz the power, 2015-04-10 04:27:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-10 04:27:08]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/5...)

[caitp (gmail), 2015-04-10 21:43:11]

The CQ bit was checked by caitpotter88@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2015-04-10 21:43:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1072573002/120001

[caitp (gmail), 2015-04-10 22:03:54]

On 2015/04/10 04:27:08, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   linux_blink_rel on tryserver.blink (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/5...)

It looks like just 3 @@toStringTag-related failures locally, just want the
dry-run to verify

It's all tests expecting O.p.toString(promise) to be `[object Object]` but now
being the spec'd value.

[caitp (gmail), 2015-04-10 22:04:20]

On 2015/04/10 22:03:54, caitp wrote:
> On 2015/04/10 04:27:08, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   linux_blink_rel on tryserver.blink (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/5...)
> 
> It looks like just 3 @@toStringTag-related failures locally, just want the
> dry-run to verify
> 
> It's all tests expecting O.p.toString(promise) to be `[object Object]` but now
> being the spec'd value.

I think some of the failures in the dryrun are fixed by the followup "delete
dead code" patch

[caitp (gmail), 2015-04-10 22:05:05]

The CQ bit was unchecked by caitpotter88@gmail.com

[caitp (gmail), 2015-04-11 00:25:21]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-04-11 00:25:52]

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

[caitp (gmail), 2015-04-11 00:26:38]

The CQ bit was unchecked by caitpotter88@gmail.com

[caitp (gmail), 2015-04-11 00:27:09]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-04-11 00:27:27]

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

[commit-bot: I haz the power, 2015-04-11 00:51:12]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-04-11 00:51:21]

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