LGTM with nits.
A higher-level nit is that I prefer test cases to be a lot smaller than 1800
lines. When trying to nail down a particular problem, that makes it
considerably
easier to find a reduced repro case.
http://codereview.chromium.org/7849021/diff/5001/test/mjsunit/harmony/proxies.js
File test/mjsunit/harmony/proxies.js (right):
http://codereview.chromium.org/7849021/diff/5001/test/mjsunit/harmony/proxies.js#newcode1721
test/mjsunit/harmony/proxies.js:1721: assertSame("", receiver)
This works, but you're implicitly testing an unrelated implementation
detail here. I'd use »assertEquals("", receiver)« instead. (Also below.)
http://codereview.chromium.org/7849021/diff/5001/test/mjsunit/harmony/proxies.js#newcode1726
test/mjsunit/harmony/proxies.js:1726: // TODO(rossberg): test once we
merge with elements branch
nit: please indent to the same level as the surrounding code. (Also at
several places below.)
http://codereview.chromium.org/7849021/diff/5001/test/mjsunit/harmony/proxies.js#newcode1755
test/mjsunit/harmony/proxies.js:1755: assertThrows(function(){ "use
strict"; o.b = 51 }, TypeError)
nit: space before '{'
http://codereview.chromium.org/7849021/diff/5001/test/mjsunit/harmony/proxies.js#newcode1774
test/mjsunit/harmony/proxies.js:1774: assertEquals(0, value) // oo has
own `a'
nit: s/`/'/
http://codereview.chromium.org/7849021/diff/5001/test/mjsunit/harmony/proxies.js#newcode1778
test/mjsunit/harmony/proxies.js:1778: assertThrows(function(){ "use
strict"; oo.b = 61 }, TypeError)
nit: space before '{'
http://codereview.chromium.org/7849021/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
