Implement IsConstructor() abstract operation

Implement IsConstructor() abstract operation

LOG=N
R=dpino@igalia.com, arv@chromium.org, dslomov@chromium.org
BUG=

Committed: https://crrev.com/6dc82c184c49c21dad8233c26ed79fcf58319fbf
Cr-Commit-Position: refs/heads/master@{#26130}

[caitp (gmail), 2015-01-17 19:45:49]

PTAL --- I realized today ArrayFrom was doing "the wrong thing" by not
supporting bound functions. In particular, it should only care about the
presence of a [[Construct]] internal slot.

Unfortunately, construct_stub doesn't offer a great way to check this, but
should_have_prototype seems to be a rough way of doing this.

It would be nice to inline this, and maybe make heuristics for checking if a
function is a constructor better.

[caitp (gmail), 2015-01-17 23:17:13]

On 2015/01/17 19:45:49, caitp wrote:
> PTAL --- I realized today ArrayFrom was doing "the wrong thing" by not
> supporting bound functions. In particular, it should only care about the
> presence of a [[Construct]] internal slot.
> 
> Unfortunately, construct_stub doesn't offer a great way to check this, but
> should_have_prototype seems to be a rough way of doing this.
> 
> It would be nice to inline this, and maybe make heuristics for checking if a
> function is a constructor better.

Mozilla's version of this is more elegant, since they can just test a single
bitfield of the JSFunction object
https://dxr.mozilla.org/mozilla-central/source/js/src/jsfun.h#31-60 --- then
there are a few flag check heuristics for figuring out constructor-status.
Something like that could be ported in, and would probably be more trustworthy
than `should_have_prototype`

[arv (Not doing code reviews), 2015-01-18 18:55:29]

On 2015/01/17 23:17:13, caitp wrote:
> On 2015/01/17 19:45:49, caitp wrote:
> > PTAL --- I realized today ArrayFrom was doing "the wrong thing" by not
> > supporting bound functions. In particular, it should only care about the
> > presence of a [[Construct]] internal slot.
> > 
> > Unfortunately, construct_stub doesn't offer a great way to check this, but
> > should_have_prototype seems to be a rough way of doing this.
> > 
> > It would be nice to inline this, and maybe make heuristics for checking if a
> > function is a constructor better.
> 
> Mozilla's version of this is more elegant, since they can just test a single
> bitfield of the JSFunction object
> https://dxr.mozilla.org/mozilla-central/source/js/src/jsfun.h#31-60 --- then
> there are a few flag check heuristics for figuring out constructor-status.
> Something like that could be ported in, and would probably be more trustworthy
> than `should_have_prototype`

Maybe we should add a bit for has_constructor (is_constructable).

[Dmitry Lomov (no reviews), 2015-01-19 10:42:41]

This patch lgtm.

I'd use HandleScope and handles instead of SHS in Runtime_IsConstructor, but I
leave it up to you to decide.

We might need is_constructable anyway, but that does not have to block this
patch,

[caitp (gmail), 2015-01-19 11:26:21]

On 2015/01/19 10:42:41, Dmitry Lomov (chromium) wrote:
> This patch lgtm.
> 
> I'd use HandleScope and handles instead of SHS in Runtime_IsConstructor, but I
> leave it up to you to decide.
> 
> We might need is_constructable anyway, but that does not have to block this
> patch,

I'll leave a TODO near the runtime function to implement it better

[caitp (gmail), 2015-01-19 11:34:55]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2015-01-19 11:35:45]

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

[Dmitry Lomov (no reviews), 2015-01-19 11:37:49]

The CQ bit was unchecked by dslomov@chromium.org

[Dmitry Lomov (no reviews), 2015-01-19 11:39:46]

Sorry the last patch not lgtm.
You are mixing up raw pointers and handles, that's not good (easy fix too)

https://codereview.chromium.org/851163007/diff/20001/src/runtime/runtime-func...
File src/runtime/runtime-function.cc (right):

https://codereview.chromium.org/851163007/diff/20001/src/runtime/runtime-func...
src/runtime/runtime-function.cc:333: CONVERT_ARG_CHECKED(Object, object, 0);
Must be CONVERT_HANDLE_ARG_CHECKED.

[caitp (gmail), 2015-01-19 11:49:14]

On 2015/01/19 11:39:46, Dmitry Lomov (chromium) wrote:
> Sorry the last patch not lgtm.
> You are mixing up raw pointers and handles, that's not good (easy fix too)
> 
>
https://codereview.chromium.org/851163007/diff/20001/src/runtime/runtime-func...
> File src/runtime/runtime-function.cc (right):
> 
>
https://codereview.chromium.org/851163007/diff/20001/src/runtime/runtime-func...
> src/runtime/runtime-function.cc:333: CONVERT_ARG_CHECKED(Object, object, 0);
> Must be CONVERT_HANDLE_ARG_CHECKED.

All raw pointers converted to handles

[caitp (gmail), 2015-01-19 11:49:15]

On 2015/01/19 11:39:46, Dmitry Lomov (chromium) wrote:
> Sorry the last patch not lgtm.
> You are mixing up raw pointers and handles, that's not good (easy fix too)
> 
>
https://codereview.chromium.org/851163007/diff/20001/src/runtime/runtime-func...
> File src/runtime/runtime-function.cc (right):
> 
>
https://codereview.chromium.org/851163007/diff/20001/src/runtime/runtime-func...
> src/runtime/runtime-function.cc:333: CONVERT_ARG_CHECKED(Object, object, 0);
> Must be CONVERT_HANDLE_ARG_CHECKED.

All raw pointers converted to handles

[Dmitry Lomov (no reviews), 2015-01-19 11:49:57]

The CQ bit was checked by dslomov@chromium.org

[Dmitry Lomov (no reviews), 2015-01-19 11:49:57]

lgtm

[commit-bot: I haz the power, 2015-01-19 11:50:45]

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

[commit-bot: I haz the power, 2015-01-19 12:22:29]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-19 12:22:39]

Patchset 3 (id:??) landed as
https://crrev.com/6dc82c184c49c21dad8233c26ed79fcf58319fbf
Cr-Commit-Position: refs/heads/master@{#26130}