Mads,
Could you have a final look.
I added index names to both runtime.cc and macros.py (using one enum in
runtime). I also changed v8natives.js to use the new indices.
In addition, I updated es5conform.status to reflect that we now support
element
indices.
http://codereview.chromium.org/2278002/diff/7001/8001
File src/runtime.cc (right):
http://codereview.chromium.org/2278002/diff/7001/8001#newcode606
src/runtime.cc:606: elms->set(0, Heap::false_value());
On 2010/05/26 13:55:12, Mads Ager wrote:
On 2010/05/26 13:43:57, Rico wrote:
> On 2010/05/26 11:23:18, Mads Ager wrote:
> > Maybe we should define named constants for 0-4. Would make it
easier to read
> the
> > code for property descriptors.
> They problem is that they represent different things depending on if
it is a
> data property or an accessor as described by the method comment. I
could make
> two enums or one with mixed names (i.e., GET_FUNCTION_OR_VALUE and
> SET_FUNCTION_OR_WRITABLE). We would still, however, have magic
numbers in
> v8natives.js.
More than one enum would be fine by me. Also, you can use macros.py to
avoid
magic numbers in v8natives.js. Feel free to do that as a followup
change though,
but it would be nice for readability.
I changed this to one enum - to be better aligned with what a
propertydescriptor actually is. This changes the length of the returned
array but I think this is a small penalty to pay for the increased
readability. I added the indices to macros.py as well and changed
v8natives to use these.
http://codereview.chromium.org/2278002/diff/8003/12001#newcode597
src/runtime.cc:597:
On 2010/05/26 13:55:12, Mads Ager wrote:
This is by -> This is handled by?
Done.
http://codereview.chromium.org/2278002/diff/8003/12002
File test/mjsunit/get-own-property-descriptor.js (right):
http://codereview.chromium.org/2278002/diff/8003/12002#newcode33
test/mjsunit/get-own-property-descriptor.js:33: function get() {return
x;}
On 2010/05/26 13:55:12, Mads Ager wrote:
Could you add a bit of spacing here? { return x; }
Done.
http://codereview.chromium.org/2278002/diff/14001/15002#newcode28
test/mjsunit/get-own-property-descriptor.js:28: // This file only tests
very simple descriptors that always have
On 2010/05/26 13:55:12, Mads Ager wrote:
Could you add testing of objects that have the element in their
prototype chain
instead of on the object itself? There was a code change to use
HasLocalElement
instead of HasElement. We should make sure there is a regression test
for that.
Done.
http://codereview.chromium.org/2278002/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
