25477 2009-04-29 23:57:01 -0700 [ES5] Sorting a non-array creates propreties (spec-violation) 2012-09-23 22:34:23 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Minor --- 1 lrn barraclough ap barraclough ggaren oldest_to_newest 119434 0 lrn 2009-04-29 23:57:01 -0700 Sorting a sparse non-array object will create properties with the value "undefined" instead of deleting existing properties. Example: [].sort.call({length:5,2:42}).hasOwnProperty(2) The resulting object has the form {length:5, 0:42, 2:undefined}. According to the specification, the "2" property should not exist. 119582 1 ap 2009-05-01 03:51:56 -0700 Do we also disagree with IE and/or Firefox here? 119584 2 lrn 2009-05-01 04:38:57 -0700 IE 7 and Firefox 3.08 both give "false" on the example expression (the expected result), where JSC gives "true". It is probably an articfact of the handling of objects with inherited array indices in Array.prototype.sort. 726247 3 165283 barraclough 2012-09-23 00:10:49 -0700 Created attachment 165283 Fix 726248 4 165283 sam 2012-09-23 00:13:12 -0700 Comment on attachment 165283 Fix Tests? 726282 5 165283 oliver 2012-09-23 12:04:14 -0700 Comment on attachment 165283 Fix r+ but are we repeatedly retrieving the methodTable? It seems like we could retrieve it just once and be done... 726299 6 ggaren 2012-09-23 15:00:52 -0700 /me agrees with Sam Weinig. 726300 7 barraclough 2012-09-23 15:29:12 -0700 (In reply to comment #5) > (From update of attachment 165283 [details]) > r+ but are we repeatedly retrieving the methodTable? It seems like we could retrieve it just once and be done... Ugh, I'm going to ignore that. :-) (1) It goes against the direction I'd like us to go for methodTable() access - we should be able to add wrappers to JSCell that automatically call via the method table, to avoid the code duplication everywhere. (2) That may not matter – the C compiler may alias the loads. We should only manually fold the methodTable() accesses if profiling indicates this is important. (3) This is array::sort applied to a non-array object – if performance matters we should throw away the internet and start again. :-) You're right, might be a useful optimization in some places – I hope not though. 726301 8 barraclough 2012-09-23 15:29:49 -0700 (In reply to comment #4) > (From update of attachment 165283 [details]) > Tests? Ooops, I did add a couple of tests, they seem not to be in the patch I added. Oh well, they'll be there when I land. :-) 726389 9 barraclough 2012-09-23 22:34:23 -0700 Fixed in r129317 165283 2012-09-23 00:10:49 -0700 2012-09-23 12:04:14 -0700 Fix 25477.patch text/plain 4296 barraclough SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTI5MzE0KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBA CisyMDEyLTA5LTIzICBHYXZpbiBCYXJyYWNsb3VnaCAgPGJhcnJhY2xvdWdoQGFwcGxlLmNvbT4K KworICAgICAgICBTb3J0aW5nIGEgbm9uLWFycmF5IGNyZWF0ZXMgcHJvcHJldGllcyAoc3BlYy12 aW9sYXRpb24pCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0yNTQ3NworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFdlJ3JlIGp1c3QgY2FsbGluZyBnZXQoKSB0byBnZXQgcHJvcGVydGllcywgd2hpY2ggaXMgY29u dmVydGluZyBtaXNzaW5nIHByb3BlcnRpZXMgdG8KKyAgICAgICAgdW5kZWZpbmVkLiBIb2xlIHZh bHVlcyBzaG91bGQgYmUgcmV0YWluZWQsIGFuZCBtb3ZlZCB0byB0aGUgZW5kIG9mIHRoZSBhcnJh eS4KKworICAgICAgICAqIHJ1bnRpbWUvQXJyYXlQcm90b3R5cGUuY3BwOgorICAgICAgICAoSlND OjpnZXRPckhvbGUpOgorICAgICAgICAgICAgLSBIZWxwZXIgZnVuY3Rpb24sIHJldHVybnMgSlNW YWx1ZSgpIGluc3RlYWQgb2YgdW5kZWZpbmVkIGZvciBtaXNzaW5nIHByb3BlcnRpZXMuCisgICAg ICAgIChKU0M6OmFycmF5UHJvdG9GdW5jU29ydCk6CisgICAgICAgICAgICAtIEltcGxlbWVudGVk IHBlciAxNS40LjQuMTEsIHNlZSBjb21tZW50cyBhYm92ZS4KKwogMjAxMi0wOS0yMSAgR2VvZmZy ZXkgR2FyZW4gIDxnZ2FyZW5AYXBwbGUuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxl ZCBvdXQgYSBsaW5lIEkgY29tbWl0dGVkIGJ5IGFjY2lkZW50LgpJbmRleDogU291cmNlL0phdmFT Y3JpcHRDb3JlL3J1bnRpbWUvQXJyYXlQcm90b3R5cGUuY3BwCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJj ZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0FycmF5UHJvdG90eXBlLmNwcAkocmV2aXNpb24gMTI5 MzA5KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvQXJyYXlQcm90b3R5cGUuY3Bw CSh3b3JraW5nIGNvcHkpCkBAIC02MjksNiArNjI5LDE1IEBAIEVuY29kZWRKU1ZhbHVlIEpTQ19I T1NUX0NBTEwgYXJyYXlQcm90b0YKICAgICByZXR1cm4gSlNWYWx1ZTo6ZW5jb2RlKHJlc3VsdCk7 CiB9CiAKK2lubGluZSBKU1ZhbHVlIGdldE9ySG9sZShKU09iamVjdCogb2JqLCBFeGVjU3RhdGUq IGV4ZWMsIHVuc2lnbmVkIHByb3BlcnR5TmFtZSkKK3sKKyAgICBQcm9wZXJ0eVNsb3Qgc2xvdChv YmopOworICAgIGlmIChvYmotPmdldFByb3BlcnR5U2xvdChleGVjLCBwcm9wZXJ0eU5hbWUsIHNs b3QpKQorICAgICAgICByZXR1cm4gc2xvdC5nZXRWYWx1ZShleGVjLCBwcm9wZXJ0eU5hbWUpOwor CisgICAgcmV0dXJuIEpTVmFsdWUoKTsKK30KKwogRW5jb2RlZEpTVmFsdWUgSlNDX0hPU1RfQ0FM TCBhcnJheVByb3RvRnVuY1NvcnQoRXhlY1N0YXRlKiBleGVjKQogewogICAgIEpTT2JqZWN0KiB0 aGlzT2JqID0gZXhlYy0+aG9zdFRoaXNWYWx1ZSgpLnRvT2JqZWN0KGV4ZWMpOwpAQCAtNjUzLDE3 ICs2NjIsMjEgQEAgRW5jb2RlZEpTVmFsdWUgSlNDX0hPU1RfQ0FMTCBhcnJheVByb3RvRgogICAg IC8vICJNaW4iIHNvcnQuIE5vdCB0aGUgZmFzdGVzdCwgYnV0IGRlZmluaXRlbHkgbGVzcyBjb2Rl IHRoYW4gaGVhcHNvcnQKICAgICAvLyBvciBxdWlja3NvcnQsIGFuZCBtdWNoIGxlc3Mgc3dhcHBp bmcgdGhhbiBidWJibGVzb3J0L2luc2VydGlvbnNvcnQuCiAgICAgZm9yICh1bnNpZ25lZCBpID0g MDsgaSA8IGxlbmd0aCAtIDE7ICsraSkgewotICAgICAgICBKU1ZhbHVlIGlPYmogPSB0aGlzT2Jq LT5nZXQoZXhlYywgaSk7CisgICAgICAgIEpTVmFsdWUgaU9iaiA9IGdldE9ySG9sZSh0aGlzT2Jq LCBleGVjLCBpKTsKICAgICAgICAgaWYgKGV4ZWMtPmhhZEV4Y2VwdGlvbigpKQogICAgICAgICAg ICAgcmV0dXJuIEpTVmFsdWU6OmVuY29kZShqc1VuZGVmaW5lZCgpKTsKICAgICAgICAgdW5zaWdu ZWQgdGhlbWluID0gaTsKICAgICAgICAgSlNWYWx1ZSBtaW5PYmogPSBpT2JqOwogICAgICAgICBm b3IgKHVuc2lnbmVkIGogPSBpICsgMTsgaiA8IGxlbmd0aDsgKytqKSB7Ci0gICAgICAgICAgICBK U1ZhbHVlIGpPYmogPSB0aGlzT2JqLT5nZXQoZXhlYywgaik7CisgICAgICAgICAgICBKU1ZhbHVl IGpPYmogPSBnZXRPckhvbGUodGhpc09iaiwgZXhlYywgaik7CiAgICAgICAgICAgICBpZiAoZXhl Yy0+aGFkRXhjZXB0aW9uKCkpCiAgICAgICAgICAgICAgICAgcmV0dXJuIEpTVmFsdWU6OmVuY29k ZShqc1VuZGVmaW5lZCgpKTsKICAgICAgICAgICAgIGRvdWJsZSBjb21wYXJlUmVzdWx0OwotICAg ICAgICAgICAgaWYgKGpPYmouaXNVbmRlZmluZWQoKSkKKyAgICAgICAgICAgIGlmICghak9iaikK KyAgICAgICAgICAgICAgICBjb21wYXJlUmVzdWx0ID0gMTsKKyAgICAgICAgICAgIGVsc2UgaWYg KCFtaW5PYmopCisgICAgICAgICAgICAgICAgY29tcGFyZVJlc3VsdCA9IC0xOworICAgICAgICAg ICAgZWxzZSBpZiAoak9iai5pc1VuZGVmaW5lZCgpKQogICAgICAgICAgICAgICAgIGNvbXBhcmVS ZXN1bHQgPSAxOyAvLyBkb24ndCBjaGVjayBtaW5PYmogYmVjYXVzZSB0aGVyZSdzIG5vIG5lZWQg dG8gZGlmZmVyZW50aWF0ZSA9PSAoMCkgZnJvbSA+ICgxKQogICAgICAgICAgICAgZWxzZSBpZiAo bWluT2JqLmlzVW5kZWZpbmVkKCkpCiAgICAgICAgICAgICAgICAgY29tcGFyZVJlc3VsdCA9IC0x OwpAQCAtNjgyLDEyICs2OTUsMjIgQEAgRW5jb2RlZEpTVmFsdWUgSlNDX0hPU1RfQ0FMTCBhcnJh eVByb3RvRgogICAgICAgICB9CiAgICAgICAgIC8vIFN3YXAgdGhlbWluIGFuZCBpCiAgICAgICAg IGlmICh0aGVtaW4gPiBpKSB7Ci0gICAgICAgICAgICB0aGlzT2JqLT5tZXRob2RUYWJsZSgpLT5w dXRCeUluZGV4KHRoaXNPYmosIGV4ZWMsIGksIG1pbk9iaiwgdHJ1ZSk7Ci0gICAgICAgICAgICBp ZiAoZXhlYy0+aGFkRXhjZXB0aW9uKCkpCisgICAgICAgICAgICBpZiAobWluT2JqKSB7CisgICAg ICAgICAgICAgICAgdGhpc09iai0+bWV0aG9kVGFibGUoKS0+cHV0QnlJbmRleCh0aGlzT2JqLCBl eGVjLCBpLCBtaW5PYmosIHRydWUpOworICAgICAgICAgICAgICAgIGlmIChleGVjLT5oYWRFeGNl cHRpb24oKSkKKyAgICAgICAgICAgICAgICAgICAgcmV0dXJuIEpTVmFsdWU6OmVuY29kZShqc1Vu ZGVmaW5lZCgpKTsKKyAgICAgICAgICAgIH0gZWxzZSBpZiAoIXRoaXNPYmotPm1ldGhvZFRhYmxl KCktPmRlbGV0ZVByb3BlcnR5QnlJbmRleCh0aGlzT2JqLCBleGVjLCBpKSkgeworICAgICAgICAg ICAgICAgIHRocm93VHlwZUVycm9yKGV4ZWMsICJVbmFibGUgdG8gZGVsZXRlIHByb3BlcnR5LiIp OwogICAgICAgICAgICAgICAgIHJldHVybiBKU1ZhbHVlOjplbmNvZGUoanNVbmRlZmluZWQoKSk7 Ci0gICAgICAgICAgICB0aGlzT2JqLT5tZXRob2RUYWJsZSgpLT5wdXRCeUluZGV4KHRoaXNPYmos IGV4ZWMsIHRoZW1pbiwgaU9iaiwgdHJ1ZSk7Ci0gICAgICAgICAgICBpZiAoZXhlYy0+aGFkRXhj ZXB0aW9uKCkpCisgICAgICAgICAgICB9CisgICAgICAgICAgICBpZiAoaU9iaikgeworICAgICAg ICAgICAgICAgIHRoaXNPYmotPm1ldGhvZFRhYmxlKCktPnB1dEJ5SW5kZXgodGhpc09iaiwgZXhl YywgdGhlbWluLCBpT2JqLCB0cnVlKTsKKyAgICAgICAgICAgICAgICBpZiAoZXhlYy0+aGFkRXhj ZXB0aW9uKCkpCisgICAgICAgICAgICAgICAgICAgIHJldHVybiBKU1ZhbHVlOjplbmNvZGUoanNV bmRlZmluZWQoKSk7CisgICAgICAgICAgICB9IGVsc2UgaWYgKCF0aGlzT2JqLT5tZXRob2RUYWJs ZSgpLT5kZWxldGVQcm9wZXJ0eUJ5SW5kZXgodGhpc09iaiwgZXhlYywgdGhlbWluKSkgeworICAg ICAgICAgICAgICAgIHRocm93VHlwZUVycm9yKGV4ZWMsICJVbmFibGUgdG8gZGVsZXRlIHByb3Bl cnR5LiIpOwogICAgICAgICAgICAgICAgIHJldHVybiBKU1ZhbHVlOjplbmNvZGUoanNVbmRlZmlu ZWQoKSk7CisgICAgICAgICAgICB9CiAgICAgICAgIH0KICAgICB9CiAgICAgcmV0dXJuIEpTVmFs dWU6OmVuY29kZSh0aGlzT2JqKTsK