Issue 546032: Enabled es5conform tests for new array methods and corrected errors that was ...

Issue 546032: Enabled es5conform tests for new array methods and corrected errors that was ... (Closed)

Created:
10 years, 11 months ago by Rico

Modified:
9 years, 6 months ago

Reviewers:
Christian Plesner Hansen

CC:
v8-dev

Base URL:
http://v8.googlecode.com/svn/branches/bleeding_edge/

Visibility:
Public.

Description

Enabled es5conform tests for new array methods and corrected errors that was discovered by enabling these. Added new es5 methods: GetOwnPropertyDescriptor GetOwnProperty FromPropertyDescriptor Committed: http://code.google.com/p/v8/source/detail?r=3595

Patch Set 1 #

Total comments: 6

Patch Set 2 : '' #

Created: 10 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+336 lines, -10 lines)			Patch
M	src/array.js	View	1	4 chunks	+7 lines, -6 lines	0 comments	Download
M	src/messages.js	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	src/runtime.h	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	src/runtime.cc	View	1	1 chunk	+61 lines, -0 lines	0 comments	Download
M	src/v8natives.js	View		4 chunks	+55 lines, -1 line	0 comments	Download
M	test/es5conform/es5conform.status	View		3 chunks	+158 lines, -2 lines	0 comments	Download
A	test/mjsunit/get-own-property-descriptor.js	View	1	1 chunk	+51 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Christian Plesner Hansen

Lgtm, with comments. http://codereview.chromium.org/546032/diff/1/7 File src/array.js (right): http://codereview.chromium.org/546032/diff/1/7#newcode986 src/array.js:986: if(IS_UNDEFINED(element)) { Space after if. http://codereview.chromium.org/546032/diff/1/4 ...

10 years, 11 months ago (2010-01-13 11:30:45 UTC) #2

Rico

10 years, 11 months ago (2010-01-13 12:08:54 UTC) #3

http://codereview.chromium.org/546032/diff/1/7
File src/array.js (right):

http://codereview.chromium.org/546032/diff/1/7#newcode986
src/array.js:986: if(IS_UNDEFINED(element)) {
On 2010/01/13 11:30:45, Christian Plesner Hansen wrote:
> Space after if.

Done.

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

http://codereview.chromium.org/546032/diff/1/4#newcode569
src/runtime.cc:569: if (obj->GetPrototype()->IsJSObject() &&
On 2010/01/13 11:30:45, Christian Plesner Hansen wrote:
> Here and below we do obj->GetPrototype() three times.  Consider fetching it
into
> a local variable just once.

Done.

http://codereview.chromium.org/546032/diff/1/8
File test/mjsunit/get-own-property-descriptor.js (right):

http://codereview.chromium.org/546032/diff/1/8#newcode43
test/mjsunit/get-own-property-descriptor.js:43:
assertTrue(descIsAccessor.configurable);
On 2010/01/13 11:30:45, Christian Plesner Hansen wrote:
> Consider adding checks that descIsAccessor.{get,set} return the expected
values.

Done.

Expand Messages | Collapse Messages