LGTM

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

http://codereview.chromium.org/7064027/diff/1/src/objects.cc#newcode7474
src/objects.cc:7474: Object* setter =
FixedArray::cast(structure)->get(kSetterIndex);
Here I notice a non handlerized pointer in what is otherwise handler
based code. Please wrap it in a handle, for consistency.

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

http://codereview.chromium.org/7064027/diff/1/test/mjsunit/getter-in-prototype.js#newcode45
test/mjsunit/getter-in-prototype.js:45: assertDoesNotThrow("f()");
Cleanup: Just do
  assertDoesNotThrow(f);
It can take both a string and a function, so the string is extra
complication

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

http://codereview.chromium.org/7064027/diff/1/test/mjsunit/regress/regress-1355.js#newcode35
test/mjsunit/regress/regress-1355.js:35: });
Or just
 var foo = {get bar() { return 10; }};

http://codereview.chromium.org/7064027/diff/1/test/mjsunit/regress/regress-1355.js#newcode44
test/mjsunit/regress/regress-1355.js:44: assertThrows("shouldThrow()");
This test doesn't seem to be as thorough as the one for not throwing -
no indexed properties/elements, no variable lookup (from with or global
object).
Please add the other cases too.

http://codereview.chromium.org/7064027/

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

Reply via email to