Made sort on non-arrays also affect elements on the prototype, for JSC compatability.

Made sort on non-arrays also affect elements on the prototype, for JSC compatability.
Made sort on non-objects with inherited elements JSC compatible.

[Lasse Reichstein, 2009-05-01 07:59:48]

Review, please.
Now passes the relevant layout test.

[Erik Corry, 2009-05-01 08:13:53]

http://codereview.chromium.org/99272/diff/1/2
File src/array.js (right):

http://codereview.chromium.org/99272/diff/1/2#newcode755
Line 755: var max = (proto_length < from) ? proto_length : from;
Should be to, not from.

http://codereview.chromium.org/99272/diff/1/2#newcode763
Line 763: for (var i = 0; i < indices.length; i++) {
Should check from and to.

[Mads Ager (chromium), 2009-05-01 08:22:22]

We need to add tests for this.  There are uncovered paths in this code and there
seems to be bugs.

http://codereview.chromium.org/99272/diff/1/2
File src/array.js (right):

http://codereview.chromium.org/99272/diff/1/2#newcode755
Line 755: var max = (proto_length < from) ? proto_length : from;
from -> to

This code does not seem to be exercised by a test.  Please make sure to add
testing of all the cases here.  This is rather subtle.

http://codereview.chromium.org/99272/diff/1/2#newcode794
Line 794: if (!is_array & num_non_undefined + 1 < max_prototype_element) {
& -> &&

Also, could you put in some parenthesis to clearify.

[Erik Corry, 2009-05-01 09:58:55]

Does it lint?

LGTM.

http://codereview.chromium.org/99272/diff/1007/9
File src/array.js (right):

http://codereview.chromium.org/99272/diff/1007/9#newcode717
Line 717: for (var o = obj.__proto__; o; o = o.__proto__) {
Can't we call o proto or something?  These 1-letter variable names make things
hard to read.

http://codereview.chromium.org/99272/diff/1007/9#newcode721
Line 721: // It's an interval
Missing full stop.

http://codereview.chromium.org/99272/diff/1007/9#newcode748
Line 748: for (var o = obj.__proto__; o; o = o.__proto__) {
And here.

http://codereview.chromium.org/99272/diff/1007/9#newcode752
Line 752: // It's an interval
And here.

http://codereview.chromium.org/99272/diff/1007/9#newcode754
Line 754: if (proto_length > from) {
>= perhaps?

http://codereview.chromium.org/99272/diff/1007/9#newcode787
Line 787: var max_prototype_element = CopyFromPrototype(this, length);
I think the var declaration should be hoisted out of the {} if you are going to
use it outside the {}.  I know JS doesn't care, but I do!

[Lasse Reichstein, 2009-05-01 10:05:39]

It lints. But only because the linter failed to catch a trailing space in the js
file.

http://codereview.chromium.org/99272/diff/1007/9
File src/array.js (right):

http://codereview.chromium.org/99272/diff/1007/9#newcode754
Line 754: if (proto_length > from) {
Wouldn't make a difference. The next line is a loop from "from" to, but not
including, "proto_length". In fact, the entire if can be dropped. If
proto_length <= from the next for loop will be a nop.