Søren,
thanks a lot for first round of comments.
http://codereview.chromium.org/650043/diff/1/3
File src/builtins.cc (right):
http://codereview.chromium.org/650043/diff/1/3#newcode390
src/builtins.cc:390: static bool ArrayPrototypeHasNoElements() {
On 2010/02/24 21:48:59, Søren Gjesse wrote:
Are these checks enough? I think you can introduce elements higher up
the chain
like this:
Array.prototype.__proto__ = {a:1}
Array.prototype.__proto__.__proto__ = {"1":2}
a = new Array()
a[1] is now 2. Are there any constraints that the array operations
will only
look one step up the chain, or am I missing something here?
I hope the checks are enough (I added another test though). The
idea---ArrayPrototypeHasNoElements() checks the whole prototype chain
(see IsNull check at the very end). One catch: if one would manage to
setup an object with indexed interceptor getter, that might be a
problem.
Alas, new check makes it slower somewhat. I have an idea how to speed
up this check, but even this slower variant is still worth committing.
Do we have a test on the non-writability of Object and Array prototype
fields?
I didn't find immediately, but added one. Is there a better place?
I'll add another for Object and probably other types as well if overall
approach is fine.
http://codereview.chromium.org/650043/diff/1/3#newcode464
src/builtins.cc:464: // TODO(antonm): try to shift/copy RSet bits when
moving/copying.
On 2010/02/24 21:48:59, Søren Gjesse wrote:
Please open an issue and change this TODO to TODO(issue).
Just removed. I'm working on it, so won't forget about it.
http://codereview.chromium.org/650043/diff/1/5
File src/objects.h (right):
http://codereview.chromium.org/650043/diff/1/5#newcode1629
src/objects.h:1629: // Gives access to raw memory which stores array's
data.
On 2010/02/24 21:48:59, Søren Gjesse wrote:
array's -> the array's
Done.
http://codereview.chromium.org/650043/diff/1/8
File test/mjsunit/array-elements-from-array-prototype.js (right):
http://codereview.chromium.org/650043/diff/1/8#newcode31
test/mjsunit/array-elements-from-array-prototype.js:31: // If add any
new tests here, consider adding them to all four files:
On 2010/02/24 21:48:59, Søren Gjesse wrote:
four == two?
Ah, yes, I planned to have four (for getters), but ended with two. Now
with three. Thanks a lot for spotting this.
http://codereview.chromium.org/650043/diff/1/8#newcode174
test/mjsunit/array-elements-from-array-prototype.js:174: // now owned
undefined resides at 4 and rest is shifted right by one
On 2010/02/24 21:48:59, Søren Gjesse wrote:
Start comment uppercase and end with period.
Done.
http://codereview.chromium.org/650043/diff/1/9
File test/mjsunit/array-elements-from-object-prototype.js (right):
http://codereview.chromium.org/650043/diff/1/9#newcode29
test/mjsunit/array-elements-from-object-prototype.js:29: // Tests below
verify that elements set on Array.prototype propagate
On 2010/02/24 21:48:59, Søren Gjesse wrote:
Array -> Object?
Done.
http://codereview.chromium.org/650043/diff/1/9#newcode31
test/mjsunit/array-elements-from-object-prototype.js:31: // If add any
new tests here, consider adding them to all four files:
On 2010/02/24 21:48:59, Søren Gjesse wrote:
four == two?
Done.
http://codereview.chromium.org/650043/diff/1/9#newcode174
test/mjsunit/array-elements-from-object-prototype.js:174: // now owned
undefined resides at 4 and rest is shifted right by one
On 2010/02/24 21:48:59, Søren Gjesse wrote:
Start comment uppercase end with period.
Done.
http://codereview.chromium.org/650043/diff/1/10
File test/mjsunit/fuzz-natives.js (right):
http://codereview.chromium.org/650043/diff/1/10#newcode152
test/mjsunit/fuzz-natives.js:152: // That must only be invoked on
Array.prototype.
On 2010/02/24 21:48:59, Søren Gjesse wrote:
That must -> This can
Done.
http://codereview.chromium.org/650043
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
