Chromium Code Reviews

Issue 3462005: Fix getOwnPropertyDescriptor() support for index properties. (Closed)

Created:
10 years, 3 months ago by Vyacheslav Egorov (Chromium)
Modified:
9 years, 7 months ago
Reviewers:
antonm, Rico
CC:
v8-dev
Visibility:
Public.

Description

Fix getOwnPropertyDescriptor() support for index properties. Add support for index properties with getters, setters or indexed interceptors. For indexed interceptor case only fix crashes, do not guarantee any semantic soundness. Separate issue opened for this http://code.google.com/p/v8/issues/detail?id=877 BUG=http://code.google.com/p/v8/issues/detail?id=874 Committed: http://code.google.com/p/v8/source/detail?r=5512

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Stats (+148 lines, -51 lines)
M src/objects.h View 1 chunk +20 lines, -1 line 0 comments
M src/objects.cc View 3 chunks +24 lines, -12 lines 1 comment
M src/runtime.cc View 1 chunk +46 lines, -38 lines 2 comments
M test/cctest/test-api.cc View 1 chunk +21 lines, -0 lines 1 comment
A test/mjsunit/regress/regress-874.js View 1 chunk +37 lines, -0 lines 0 comments

Messages

Total messages: 3 (0 generated)
Vyacheslav Egorov (Chromium)
10 years, 3 months ago (2010-09-22 13:16:22 UTC) #1
antonm
Drive-bys http://codereview.chromium.org/3462005/diff/1/4 File src/runtime.cc (right): http://codereview.chromium.org/3462005/diff/1/4#newcode671 src/runtime.cc:671: elms->set(ENUMERABLE_INDEX, Heap::true_value()); I am not sure all intercepted ...
10 years, 3 months ago (2010-09-22 13:26:38 UTC) #2
Rico
10 years, 3 months ago (2010-09-22 15:23:56 UTC) #3
Slava,
I am not a 100% clear on which cases we could hit with interceptors and what the
semantics should be. But other than that LGTM

http://codereview.chromium.org/3462005/diff/1/2
File src/objects.cc (right):

http://codereview.chromium.org/3462005/diff/1/2#newcode5962
src/objects.cc:5962: 
remove blank line?

http://codereview.chromium.org/3462005/diff/1/4
File src/runtime.cc (right):

http://codereview.chromium.org/3462005/diff/1/4#newcode685
src/runtime.cc:685: } else {
Add comment on why we would go in here

Powered by Google App Engine