[proxies] throw TypeError in [[Call]] if is_callable Map bit is unset

[proxies] throw TypeError if is_callable Map bit is unset

Per ProxyCreate() (https://tc39.github.io/ecma262/#sec-proxycreate), a Proxy
is only given a [[Call]] slot if the target has a [[Call]] slot as well. This
was previously implemented correctly for [[Construct]], but not for [[Call]].

BUG=v8:4797, v8:4796, v8:1543
LOG=N
R=cbruni@chromium.org, neis@chromium.org, adamk@chromium.org, littledan@chromium.org

Committed: https://crrev.com/18b9c1ce705e5fe38c23bdd19fcbbfbe3121c611
Cr-Commit-Position: refs/heads/master@{#34461}

[caitp (gmail), 2016-03-02 16:29:41]

Hi, PTAL

[caitp (gmail), 2016-03-02 16:30:02]

Description was changed from

==========
[proxies] throw TypeError if is_callable Map bit is unset

Per ProxyCreate() (https://tc39.github.io/ecma262/#sec-proxycreate), a Proxy
is only given a [[Call]] slot if the target has a [[Call]] slot as well. This
was previously implemented correctly for [[Construct]], but not for [[Call]].

BUG=v8:4797, v8:4796, v8:1543
LOG=N
R=cbruni@chromium.org, neis@chromium.org, adamk@chromium.org,
littledan@chromium.org
==========

to

==========
[proxies] throw TypeError if is_callable Map bit is unset

Per ProxyCreate() (https://tc39.github.io/ecma262/#sec-proxycreate), a Proxy
is only given a [[Call]] slot if the target has a [[Call]] slot as well. This
was previously implemented correctly for [[Construct]], but not for [[Call]].

BUG=v8:4797, v8:4796, v8:1543
LOG=N
R=cbruni@chromium.org, neis@chromium.org, adamk@chromium.org,
littledan@chromium.org
==========

[Camillo Bruni, 2016-03-02 17:41:03]

https://codereview.chromium.org/1752133004/diff/20001/src/runtime/runtime-pro...
File src/runtime/runtime-proxy.cc (right):

https://codereview.chromium.org/1752133004/diff/20001/src/runtime/runtime-pro...
src/runtime/runtime-proxy.cc:29: isolate,
NewTypeError(MessageTemplate::kCalledNonCallable, callsite));
Would you mind fixing it directly in PLATFORM/builtins-PLATFORM.cc Generate_Call
should be fairly straight forward?
This solution will probably throw in the wrong context...

https://codereview.chromium.org/1752133004/diff/20001/test/mjsunit/harmony/pr...
File test/mjsunit/harmony/proxies-apply.js (right):

https://codereview.chromium.org/1752133004/diff/20001/test/mjsunit/harmony/pr...
test/mjsunit/harmony/proxies-apply.js:110: })();
silly that we forgot this case :)

[caitp (gmail), 2016-03-02 17:42:05]

On 2016/03/02 17:41:03, cbruni wrote:
>
https://codereview.chromium.org/1752133004/diff/20001/src/runtime/runtime-pro...
> File src/runtime/runtime-proxy.cc (right):
> 
>
https://codereview.chromium.org/1752133004/diff/20001/src/runtime/runtime-pro...
> src/runtime/runtime-proxy.cc:29: isolate,
> NewTypeError(MessageTemplate::kCalledNonCallable, callsite));
> Would you mind fixing it directly in PLATFORM/builtins-PLATFORM.cc
Generate_Call
> should be fairly straight forward?
> This solution will probably throw in the wrong context...
> 
>
https://codereview.chromium.org/1752133004/diff/20001/test/mjsunit/harmony/pr...
> File test/mjsunit/harmony/proxies-apply.js (right):
> 
>
https://codereview.chromium.org/1752133004/diff/20001/test/mjsunit/harmony/pr...
> test/mjsunit/harmony/proxies-apply.js:110: })();
> silly that we forgot this case :)

Will do

[caitp (gmail), 2016-03-02 18:26:57]

https://codereview.chromium.org/1752133004/diff/20001/src/runtime/runtime-pro...
File src/runtime/runtime-proxy.cc (right):

https://codereview.chromium.org/1752133004/diff/20001/src/runtime/runtime-pro...
src/runtime/runtime-proxy.cc:29: isolate,
NewTypeError(MessageTemplate::kCalledNonCallable, callsite));
On 2016/03/02 17:41:03, cbruni wrote:
> Would you mind fixing it directly in PLATFORM/builtins-PLATFORM.cc
Generate_Call
> should be fairly straight forward?
> This solution will probably throw in the wrong context...

Done, with ports for ia32, x64, arm and arm64

[Camillo Bruni, 2016-03-02 18:31:11]

lgtm

[caitp (gmail), 2016-03-02 18:33:37]

On 2016/03/02 18:31:11, cbruni wrote:
> lgtm

Could you leave an FYI for the mips/ppc/x87/etc people? My comments don't show
up on the v8-reviews list, and they might miss it since it's not explicitly
about codegen

[caitp (gmail), 2016-03-03 12:34:29]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2016-03-03 12:34:41]

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

[commit-bot: I haz the power, 2016-03-03 12:38:10]

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

[commit-bot: I haz the power, 2016-03-03 12:38:11]

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

[caitp (gmail), 2016-03-03 12:59:45]

caitpotter88@gmail.com changed reviewers:
+ bmeurer@chromium.org

[caitp (gmail), 2016-03-03 13:00:49]

+ bmeurer to check moving the kIsCallable bit test in builtins-PLATFORM

[Benedikt Meurer, 2016-03-03 13:19:34]

You need to provide mips/mips64 ports as well or ask the mips people.

https://codereview.chromium.org/1752133004/diff/40001/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):

https://codereview.chromium.org/1752133004/diff/40001/src/arm/builtins-arm.cc...
src/arm/builtins-arm.cc:2266: __ cmp(r5, Operand(JS_BOUND_FUNCTION_TYPE));
Bound functions are always callable, so you can move the callable check down
right before the proxy check; same for the other archs.

[caitp (gmail), 2016-03-03 13:54:16]

On 2016/03/03 13:19:34, Benedikt Meurer wrote:
> You need to provide mips/mips64 ports as well or ask the mips people.
> 
> https://codereview.chromium.org/1752133004/diff/40001/src/arm/builtins-arm.cc
> File src/arm/builtins-arm.cc (right):
> 
>
https://codereview.chromium.org/1752133004/diff/40001/src/arm/builtins-arm.cc...
> src/arm/builtins-arm.cc:2266: __ cmp(r5, Operand(JS_BOUND_FUNCTION_TYPE));
> Bound functions are always callable, so you can move the callable check down
> right before the proxy check; same for the other archs.

added MIPS ports and moved the bit test to after the bound function test

[Benedikt Meurer, 2016-03-03 13:58:44]

Thanks. LGTM!

[caitp (gmail), 2016-03-03 15:21:10]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2016-03-03 15:21:11]

The patchset sent to the CQ was uploaded after l-g-t-m from cbruni@chromium.org
Link to the patchset: https://codereview.chromium.org/1752133004/#ps100001
(title: "move bit test for MIPS/MIPS64 ports, too")

[commit-bot: I haz the power, 2016-03-03 15:21:14]

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

[commit-bot: I haz the power, 2016-03-03 15:22:56]

Description was changed from

==========
[proxies] throw TypeError if is_callable Map bit is unset

Per ProxyCreate() (https://tc39.github.io/ecma262/#sec-proxycreate), a Proxy
is only given a [[Call]] slot if the target has a [[Call]] slot as well. This
was previously implemented correctly for [[Construct]], but not for [[Call]].

BUG=v8:4797, v8:4796, v8:1543
LOG=N
R=cbruni@chromium.org, neis@chromium.org, adamk@chromium.org,
littledan@chromium.org
==========

to

==========
[proxies] throw TypeError if is_callable Map bit is unset

Per ProxyCreate() (https://tc39.github.io/ecma262/#sec-proxycreate), a Proxy
is only given a [[Call]] slot if the target has a [[Call]] slot as well. This
was previously implemented correctly for [[Construct]], but not for [[Call]].

BUG=v8:4797, v8:4796, v8:1543
LOG=N
R=cbruni@chromium.org, neis@chromium.org, adamk@chromium.org,
littledan@chromium.org
==========

[commit-bot: I haz the power, 2016-03-03 15:22:57]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-03-03 15:23:13]

Description was changed from

==========
[proxies] throw TypeError if is_callable Map bit is unset

Per ProxyCreate() (https://tc39.github.io/ecma262/#sec-proxycreate), a Proxy
is only given a [[Call]] slot if the target has a [[Call]] slot as well. This
was previously implemented correctly for [[Construct]], but not for [[Call]].

BUG=v8:4797, v8:4796, v8:1543
LOG=N
R=cbruni@chromium.org, neis@chromium.org, adamk@chromium.org,
littledan@chromium.org
==========

to

==========
[proxies] throw TypeError if is_callable Map bit is unset

Per ProxyCreate() (https://tc39.github.io/ecma262/#sec-proxycreate), a Proxy
is only given a [[Call]] slot if the target has a [[Call]] slot as well. This
was previously implemented correctly for [[Construct]], but not for [[Call]].

BUG=v8:4797, v8:4796, v8:1543
LOG=N
R=cbruni@chromium.org, neis@chromium.org, adamk@chromium.org,
littledan@chromium.org

Committed: https://crrev.com/18b9c1ce705e5fe38c23bdd19fcbbfbe3121c611
Cr-Commit-Position: refs/heads/master@{#34461}
==========

[commit-bot: I haz the power, 2016-03-03 15:23:14]

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