LGTM

http://codereview.chromium.org/6451004/diff/1/src/messages.cc
File src/messages.cc (right):

http://codereview.chromium.org/6451004/diff/1/src/messages.cc#newcode78
src/messages.cc:78: Factory::NewJSArrayWithElements(arguments_elements);
Is this fixing anything, or is it just a rewrite?

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

http://codereview.chromium.org/6451004/diff/1/src/objects.cc#newcode6967
src/objects.cc:6967: if (result != Heap::the_hole_value()) return
result;
Check if the result is a failure instead.
You are comparing pointers of different type here (which is slightly
icky, even if it's true).

http://codereview.chromium.org/6451004/diff/1/src/objects.cc#newcode7102
src/objects.cc:7102: if (result != Heap::the_hole_value()) return
result;
Same here. Check result->IsFailure()

http://codereview.chromium.org/6451004/diff/1/test/mjsunit/getter-in-prototype.js
File test/mjsunit/getter-in-prototype.js (right):

http://codereview.chromium.org/6451004/diff/1/test/mjsunit/getter-in-prototype.js#newcode58
test/mjsunit/getter-in-prototype.js:58: this[0] = 42;
I would think "this" here refers to the global object, not to "o". Can
you say why this works?

http://codereview.chromium.org/6451004/diff/1/test/mjsunit/regress/regress-1107.js
File test/mjsunit/regress/regress-1107.js (right):

http://codereview.chromium.org/6451004/diff/1/test/mjsunit/regress/regress-1107.js#newcode32
test/mjsunit/regress/regress-1107.js:32: assertThrows("x.x");
Should the property name be "x", not 0?

http://codereview.chromium.org/6451004/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to