[runtime] [proxy] implementing [[Get]] trap.

[runtime] [proxy] implementing [[Get]] trap.

BUG=v8:1543
LOG=N

Committed: https://crrev.com/7e8fa4b96a02fcb0e21e13e8439b2e7aa33a3703
Cr-Commit-Position: refs/heads/master@{#32466}

[Camillo Bruni, 2015-11-30 12:34:38]

cbruni@chromium.org changed reviewers:
+ jkummerow@chromium.org, neis@chromium.org

[Jakob Kummerow, 2015-11-30 12:56:11]

LGTM with comments.

Please also lower-case the file name of getPrototypeOf.js (either with or
without dashes between words, I don't care).

https://codereview.chromium.org/1482283002/diff/1/src/contexts.h
File src/contexts.h (left):

https://codereview.chromium.org/1482283002/diff/1/src/contexts.h#oldcode111
src/contexts.h:111: V(DERIVED_GET_TRAP_INDEX, JSFunction, derived_get_trap)     
               \
I love it when this happens :-)

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

https://codereview.chromium.org/1482283002/diff/1/src/objects.cc#newcode732
src/objects.cc:732: if (JSProxy::IsRevoked(proxy)) {
this needs rebasing, and will become: if (proxy->IsRevoked()) {

https://codereview.chromium.org/1482283002/diff/1/src/objects.cc#newcode785
src/objects.cc:785: NewTypeError(MessageTemplate::kObserveAccessChecked,
trap_name),
You are technically correct that this is *a* TypeError. However, I'd suggest
that you throw *another* TypeError. (Maybe after rebasing onto
https://codereview.chromium.org/1481103002/diff/1/src/messages.h?)

https://codereview.chromium.org/1482283002/diff/1/test/mjsunit/harmony/proxy-...
File test/mjsunit/harmony/proxy-get.js (right):

https://codereview.chromium.org/1482283002/diff/1/test/mjsunit/harmony/proxy-...
test/mjsunit/harmony/proxy-get.js:23: 
How about test cases for the code paths that throw?

[Camillo Bruni, 2015-11-30 14:54:42]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-30 14:54:52]

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

[Camillo Bruni, 2015-11-30 14:57:35]

waiting for https://codereview.chromium.org/1481103002 to land

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

https://codereview.chromium.org/1482283002/diff/1/src/objects.cc#newcode732
src/objects.cc:732: if (JSProxy::IsRevoked(proxy)) {
On 2015/11/30 at 12:56:10, Jakob wrote:
> this needs rebasing, and will become: if (proxy->IsRevoked()) {

done.

https://codereview.chromium.org/1482283002/diff/1/src/objects.cc#newcode785
src/objects.cc:785: NewTypeError(MessageTemplate::kObserveAccessChecked,
trap_name),
On 2015/11/30 at 12:56:10, Jakob wrote:
> You are technically correct that this is *a* TypeError. However, I'd suggest
that you throw *another* TypeError. (Maybe after rebasing onto
https://codereview.chromium.org/1481103002/diff/1/src/messages.h?)

doh!

https://codereview.chromium.org/1482283002/diff/1/test/mjsunit/harmony/proxy-...
File test/mjsunit/harmony/proxy-get.js (right):

https://codereview.chromium.org/1482283002/diff/1/test/mjsunit/harmony/proxy-...
test/mjsunit/harmony/proxy-get.js:23: 
On 2015/11/30 at 12:56:11, Jakob wrote:
> How about test cases for the code paths that throw?

right, let's add some more detailed tests.

[commit-bot: I haz the power, 2015-11-30 15:12:51]

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

[commit-bot: I haz the power, 2015-11-30 15:12:52]

Dry run: This issue passed the CQ dry run.

[Jakob Kummerow, 2015-11-30 16:14:29]

patch set 4 LGTM with nit.

https://codereview.chromium.org/1482283002/diff/60001/test/mjsunit/harmony/pr...
File test/mjsunit/harmony/proxies-get.js (right):

https://codereview.chromium.org/1482283002/diff/60001/test/mjsunit/harmony/pr...
test/mjsunit/harmony/proxies-get.js:75: // will return a value, according to the
spec we do not throw he
he?

[neis, 2015-12-01 12:10:07]

On 2015/11/30 12:56:11, Jakob wrote:
> LGTM with comments.
> 
> Please also lower-case the file name of getPrototypeOf.js (either with or
> without dashes between words, I don't care).

Please also use "proxies-" rather than "proxy-" as prefix.

[neis, 2015-12-01 12:10:36]

On 2015/12/01 12:10:07, neis wrote:
> On 2015/11/30 12:56:11, Jakob wrote:
> > LGTM with comments.
> > 
> > Please also lower-case the file name of getPrototypeOf.js (either with or
> > without dashes between words, I don't care).
> 
> Please also use "proxies-" rather than "proxy-" as prefix.

Never mind, looked at a deleted file.

[Camillo Bruni, 2015-12-01 13:11:30]

The CQ bit was checked by cbruni@chromium.org

[Camillo Bruni, 2015-12-01 13:11:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1482283002/#ps160001
(title: "position of static")

[commit-bot: I haz the power, 2015-12-01 13:11:36]

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

[commit-bot: I haz the power, 2015-12-01 14:04:20]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-12-01 14:04:52]

Description was changed from

==========
[runtime] [proxy] implementing [[Get]] trap.

BUG=v8:1543
LOG=N
==========

to

==========
[runtime] [proxy] implementing [[Get]] trap.

BUG=v8:1543
LOG=N

Committed: https://crrev.com/7e8fa4b96a02fcb0e21e13e8439b2e7aa33a3703
Cr-Commit-Position: refs/heads/master@{#32466}
==========

[commit-bot: I haz the power, 2015-12-01 14:04:53]

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