[es6] Initial steps towards a correct implementation of IsCallable.

[es6] Initial steps towards a correct implementation of IsCallable.

This turns the has_instance_call_handler bit on Map into an is_callable
bit, that matches the spec definition of IsCallable (i.e. instances have
[[Call]] internal methods).

Also fix the typeof operator to properly say "function" for everything
that is callable.

Also remove the (unused) premature %_GetPrototype optimization from
Crankshaft, which just complicated the Map bit swap.

R=mstarzinger@chromium.org, rossberg@chromium.org, yangguo@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux_layout_dbg

Committed: https://crrev.com/8a378f46d52ce64578c71313ed76a67592fbf63c
Cr-Commit-Position: refs/heads/master@{#30552}

[Benedikt Meurer, 2015-08-26 12:27:20]

Hey Michi, Yang, Andreas,

Please review the x64 port. I'll do the ports afterwards.

Thanks,
Benedikt

[rossberg, 2015-08-26 12:55:48]

Looks mostly good to me, I just wonder how confident we can be that using
IS_CALLABLE everywhere doesn't allow tons of cases we don't actually handle.

https://codereview.chromium.org/1316933002/diff/1/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/1316933002/diff/1/src/objects-debug.cc#newcode39
src/objects-debug.cc:39: CHECK(!IsCallable());
Do we want to check this condition in more cases?

https://codereview.chromium.org/1316933002/diff/1/src/promise.js
File src/promise.js (right):

https://codereview.chromium.org/1316933002/diff/1/src/promise.js#newcode262
src/promise.js:262: : PromiseIdResolveHandler;
Nit: should fit one line now (same below)

https://codereview.chromium.org/1316933002/diff/1/src/runtime/runtime-classes.cc
File src/runtime/runtime-classes.cc (right):

https://codereview.chromium.org/1316933002/diff/1/src/runtime/runtime-classes...
src/runtime/runtime-classes.cc:107: } else if (super_class->IsJSFunction()) {
Shouldn't this be IsCallable as well? It's perfectly legal to inherit from, say,
a function proxy.

[Benedikt Meurer, 2015-08-27 05:18:24]

I'm checking the IS_CALLABLE replacements now. Most of them seem to be perfectly
fine. But I guess there'll be weird corner cases where clusterfuzz will tell us
that we did it wrong so far :-)

https://codereview.chromium.org/1316933002/diff/1/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/1316933002/diff/1/src/objects-debug.cc#newcode39
src/objects-debug.cc:39: CHECK(!IsCallable());
Don't know, but doable if you want, yeah.

https://codereview.chromium.org/1316933002/diff/1/src/promise.js
File src/promise.js (right):

https://codereview.chromium.org/1316933002/diff/1/src/promise.js#newcode262
src/promise.js:262: : PromiseIdResolveHandler;
On 2015/08/26 12:55:48, rossberg wrote:
> Nit: should fit one line now (same below)

Done.

https://codereview.chromium.org/1316933002/diff/1/src/runtime/runtime-classes.cc
File src/runtime/runtime-classes.cc (right):

https://codereview.chromium.org/1316933002/diff/1/src/runtime/runtime-classes...
src/runtime/runtime-classes.cc:107: } else if (super_class->IsJSFunction()) {
Actually it should be IsConstructor, but we don't have that yet. Plus there's a
JSFunction cast on the next line. So I'll add a TODO and clean it up later once
we have proper IsConstructor.

[Benedikt Meurer, 2015-08-27 07:21:18]

MIPS people: Please add MIPS/MIPS64 ports of the optimization for typeof in
fullcodegen and crankshaft.

[Benedikt Meurer, 2015-08-27 07:21:54]

Patchset #4 (id:60001) has been deleted

[Yang, 2015-08-27 08:39:18]

LGTM. I haven't checked for spec-compliance. Please also run layout tests.

https://codereview.chromium.org/1316933002/diff/80001/src/hydrogen-instructio...
File src/hydrogen-instructions.cc (right):

https://codereview.chromium.org/1316933002/diff/80001/src/hydrogen-instructio...
src/hydrogen-instructions.cc:2740: bit_field_ =
IsCallableField::update(bit_field_, map->is_callable());
Do we already use this field?

[Benedikt Meurer, 2015-08-27 08:39:54]

https://codereview.chromium.org/1316933002/diff/80001/src/hydrogen-instructio...
File src/hydrogen-instructions.cc (right):

https://codereview.chromium.org/1316933002/diff/80001/src/hydrogen-instructio...
src/hydrogen-instructions.cc:2740: bit_field_ =
IsCallableField::update(bit_field_, map->is_callable());
Yep in TypeOfString in the .cc file.

[Michael Starzinger, 2015-08-27 10:47:14]

LGTM if all the changes to the .js file are working as intended from a
specification perspective, didn't check those.

https://codereview.chromium.org/1316933002/diff/80001/src/execution.h
File src/execution.h (right):

https://codereview.chromium.org/1316933002/diff/80001/src/execution.h#newcode119
src/execution.h:119: MUST_USE_RESULT static Handle<String>
RenderCallSite(Isolate* isolate,
nit: If the return value is Handle<> and not MaybeHandle<>, then not using the
result should be fine.

https://codereview.chromium.org/1316933002/diff/80001/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/1316933002/diff/80001/src/objects-debug.cc#ne...
src/objects-debug.cc:39: CHECK(!IsCallable());
This CHECK does not really verify the Smi value, but is testing the IsCallable
predicate. Do we really want to have this? This seems more like something that
should be in some unittest somewhere.

https://codereview.chromium.org/1316933002/diff/80001/src/runtime/runtime-cla...
File src/runtime/runtime-classes.cc (right):

https://codereview.chromium.org/1316933002/diff/80001/src/runtime/runtime-cla...
src/runtime/runtime-classes.cc:82: Handle<Symbol>
home_object_symbol(isolate->factory()->home_object_symbol());
nit: Use assignment instead of constructor for this.

https://codereview.chromium.org/1316933002/diff/80001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/1316933002/diff/80001/test/cctest/test-api.cc...
test/cctest/test-api.cc:9799: strcmp("TypeError: object is not a function",
*exception_value2));
This will most likely hit blink layout tests. Better run one of the try-jobs
before landing.

[akos.palfi.imgtec, 2015-08-27 20:45:06]

On 2015/08/27 07:21:18, Benedikt Meurer wrote:
> MIPS people: Please add MIPS/MIPS64 ports of the optimization for typeof in
> fullcodegen and crankshaft.

Hi Benedikt,

I've uploaded the MIPS port: https://codereview.chromium.org/1310233004/

[Benedikt Meurer, 2015-09-01 09:42:47]

It looks like the layout test failure on
fast/dom/plugin-attributes-enumeration.html was indeed a bug in V8 that is now
fixed (we returned "object" for typeof applied to an API object with [[Call]]).
I checked firefox and with this patch we are compatible. So I'll fix the
expectation in Blink.

[adamk, 2015-09-01 19:20:51]

adamk@chromium.org changed reviewers:
+ adamk@chromium.org

[adamk, 2015-09-01 19:20:52]

Given that the new behavior matches FF, this lgtm. For posterity, here's why
those results change (this is the current behavior in FF, and the new behavior
in Chrome):

> typeof document.createElement('object')
"function"
> typeof document.createElement('embed')
"function"
> typeof document.createElement('applet')
"object"

Leading to a new JavaScript WAT: not only is typeof Object "function", but
typeof <object> is also "function".

[Benedikt Meurer, 2015-09-03 05:20:04]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2015-09-03 05:20:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, yangguo@chromium.org, adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/1316933002/#ps140001
(title: "Rebase again.")

[commit-bot: I haz the power, 2015-09-03 05:20:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1316933002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1316933002/140001

[commit-bot: I haz the power, 2015-09-03 06:01:23]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2015-09-03 06:01:44]

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