LGTM too.
http://codereview.chromium.org/7067019/diff/1/src/builtins.cc File src/builtins.cc (right): http://codereview.chromium.org/7067019/diff/1/src/builtins.cc#newcode369 src/builtins.cc:369: // This method depends on non writability of Object and Array prototype Sigh, but this comment should now go away. I wonder why I thought that way, probably it's ECMAScript 5 which doesn't allow to play with protos. And we cannot make them not-writable for bug-compatibility I gather :( http://codereview.chromium.org/7067019/diff/1/src/builtins.cc#newcode379 src/builtins.cc:379: if (array_proto != global_context->initial_object_prototype()) return false; while you're here, may you swap these two lines? I am concerned with the following case: Array.prototype.__proto__ = 2; http://codereview.chromium.org/7067019/diff/1/test/mjsunit/regress/regress-1403.js File test/mjsunit/regress/regress-1403.js (right): http://codereview.chromium.org/7067019/diff/1/test/mjsunit/regress/regress-1403.js#newcode28 test/mjsunit/regress/regress-1403.js:28: // See: http://code.google.com/p/v8/issues/detail?id=1403 do we have similar test case when Array.prototype.__proto__ is mutated? http://codereview.chromium.org/7067019/diff/1/test/mjsunit/regress/regress-1403.js#newcode30 test/mjsunit/regress/regress-1403.js:30: a = new Array(); nit: a = [] might be easier to read http://codereview.chromium.org/7067019/diff/1/test/mjsunit/regress/regress-1403.js#newcode33 test/mjsunit/regress/regress-1403.js:33: Object.prototype.__proto__ = p; nit: maybe (for compactness): Object.prototype.__proto__ = { __proto__: null }; http://codereview.chromium.org/7067019/diff/1/test/mjsunit/regress/regress-1403.js#newcode34 test/mjsunit/regress/regress-1403.js:34: a.shift(); I would consider adding indexed properties on Object.prototype.__proto__ to check they are handled correctly. http://codereview.chromium.org/7067019/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
