102326 2012-11-14 21:25:03 -0800 Structure should be able to easily tell if the prototype chain might intercept a store 2012-11-15 12:23:01 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 102292 102327 1 fpizlo fpizlo barraclough ggaren mark.lam mhahnenberg msaboff oliver sam webkit.review.bot oldest_to_newest 767686 0 fpizlo 2012-11-14 21:25:03 -0800 Currently all of that logic is in JSObject::put(). The logic in JSObject::put() is great, because it's well optimized for common cases, but (a) the optimizations are sometimes wrong (see https://bugs.webkit.org/show_bug.cgi?id=102292) and (b) it means that given just the structure, you can't predict if interception will happen. 767694 1 174337 fpizlo 2012-11-14 21:35:10 -0800 Created attachment 174337 the patch 767700 2 webkit.review.bot 2012-11-14 21:38:59 -0800 Attachment 174337 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/runtime/JSObject.cpp:368: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style. 767708 3 fpizlo 2012-11-14 21:47:23 -0800 (In reply to comment #2) > Attachment 174337 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 > Source/JavaScriptCore/runtime/JSObject.cpp:368: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] > Total errors found: 1 in 4 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. I kind of feel like the style I used here is appropriate. I can change it (by not wrapping the line), but I quite like it. Basically I have: if (somethingLong(moreLongThings) && anotherThing) foo; And I wrapped it to: if (somethingLong( moreLongThings) && anotherThing) foo; While the style checker wants: if (somethingLong( moreLongThings) && anotherThing) foo; To me, the latter is super awkward while the former (what I did) is quite natural. If the reviewer wants it changed, I'll change it. Otherwise I'll consider this a random case of the style checker not being awesome. 768374 4 174337 ggaren 2012-11-15 12:07:36 -0800 Comment on attachment 174337 the patch r=me Please do the wrapped line in the current style way. I think most people see this particular case as a bad case in our current guideline, so maybe you can suggest this new way in email, and persuade people. 768376 5 fpizlo 2012-11-15 12:10:32 -0800 (In reply to comment #4) > (From update of attachment 174337 [details]) > r=me > > Please do the wrapped line in the current style way. > > I think most people see this particular case as a bad case in our current guideline, so maybe you can suggest this new way in email, and persuade people. I'll just unwrap the line. Your argument that the current style reflects what most auto-indenters do is pretty convincing to me. 768381 6 fpizlo 2012-11-15 12:23:01 -0800 Landed (with style fix) in http://trac.webkit.org/changeset/134813 174337 2012-11-14 21:35:10 -0800 2012-11-15 12:07:36 -0800 the patch protochain.patch text/plain 6797 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTM0NzM1KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIzIEBA CisyMDEyLTExLTE0ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg U3RydWN0dXJlIHNob3VsZCBiZSBhYmxlIHRvIGVhc2lseSB0ZWxsIGlmIHRoZSBwcm90b3R5cGUg Y2hhaW4gbWlnaHQgaW50ZXJjZXB0IGEgc3RvcmUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTEwMjMyNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFRoaXMgaW1wcm92ZXMgb3VyIGFiaWxpdHkgdG8gcmVhc29u IGFib3V0IHRoZSBjb3JyZWN0bmVzcyBvZiB0aGUgbW9yZSBvcHRpbWl6ZWQKKyAgICAgICAgcHJv dG90eXBlIGNoYWluIHdhbGsgaW4gSlNPYmplY3Q6OnB1dCgpLCB3aGlsZSBhbHNvIG1ha2luZyBp dCBzdHJhaWdodCBmb3J3YXJkIHRvCisgICAgICAgIGNoZWNrIGlmIHRoZSBwcm90b3R5cGUgY2hh aW4gd2lsbCBkbyBzdHJhbmdlIHRoaW5ncyB0byBhIHByb3BlcnR5IHN0b3JlIGJ5IGp1c3QKKyAg ICAgICAgbG9va2luZyBhdCB0aGUgc3RydWN0dXJlLgorCisgICAgICAgICogcnVudGltZS9KU09i amVjdC5jcHA6CisgICAgICAgIChKU0M6OkpTT2JqZWN0OjpwdXQpOgorICAgICAgICAqIHJ1bnRp bWUvU3RydWN0dXJlLmNwcDoKKyAgICAgICAgKEpTQzo6U3RydWN0dXJlOjpwcm90b3R5cGVDaGFp bk1heUludGVyY2VwdFN0b3JlVG8pOgorICAgICAgICAoSlNDKToKKyAgICAgICAgKiBydW50aW1l L1N0cnVjdHVyZS5oOgorICAgICAgICAoU3RydWN0dXJlKToKKwogMjAxMi0xMS0xNCAgQnJlbnQg RnVsZ2hhbSAgPGJmdWxnaGFtQHdlYmtpdC5vcmc+CiAKICAgICAgICAgW1dpbmRvd3MsIFdpbkNh aXJvXSBVbnJldmlld2VkIGJ1aWxkIGZpeC4KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9y dW50aW1lL0pTT2JqZWN0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUv cnVudGltZS9KU09iamVjdC5jcHAJKHJldmlzaW9uIDEzNDY5NSkKKysrIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9ydW50aW1lL0pTT2JqZWN0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMzU2LDI2ICsz NTYsMzEgQEAgdm9pZCBKU09iamVjdDo6cHV0KEpTQ2VsbCogY2VsbCwgRXhlY1N0YQogICAgICAg ICBwdXRCeUluZGV4KHRoaXNPYmplY3QsIGV4ZWMsIGksIHZhbHVlLCBzbG90LmlzU3RyaWN0TW9k ZSgpKTsKICAgICAgICAgcmV0dXJuOwogICAgIH0KLQorICAgIAogICAgIC8vIENoZWNrIGlmIHRo ZXJlIGFyZSBhbnkgc2V0dGVycyBvciBnZXR0ZXJzIGluIHRoZSBwcm90b3R5cGUgY2hhaW4KICAg ICBKU1ZhbHVlIHByb3RvdHlwZTsKICAgICBpZiAocHJvcGVydHlOYW1lICE9IGV4ZWMtPnByb3Bl cnR5TmFtZXMoKS51bmRlcnNjb3JlUHJvdG8pIHsKICAgICAgICAgZm9yIChKU09iamVjdCogb2Jq ID0gdGhpc09iamVjdDsgIW9iai0+c3RydWN0dXJlKCktPmhhc1JlYWRPbmx5T3JHZXR0ZXJTZXR0 ZXJQcm9wZXJ0aWVzRXhjbHVkaW5nUHJvdG8oKTsgb2JqID0gYXNPYmplY3QocHJvdG90eXBlKSkg ewogICAgICAgICAgICAgcHJvdG90eXBlID0gb2JqLT5wcm90b3R5cGUoKTsKICAgICAgICAgICAg IGlmIChwcm90b3R5cGUuaXNOdWxsKCkpIHsKLSAgICAgICAgICAgICAgICBpZiAoIXRoaXNPYmpl Y3QtPnB1dERpcmVjdEludGVybmFsPFB1dE1vZGVQdXQ+KGdsb2JhbERhdGEsIHByb3BlcnR5TmFt ZSwgdmFsdWUsIDAsIHNsb3QsIGdldENhbGxhYmxlT2JqZWN0KHZhbHVlKSkgJiYgc2xvdC5pc1N0 cmljdE1vZGUoKSkKKyAgICAgICAgICAgICAgICBBU1NFUlQoIXRoaXNPYmplY3QtPnN0cnVjdHVy ZSgpLT5wcm90b3R5cGVDaGFpbk1heUludGVyY2VwdFN0b3JlVG8oZXhlYy0+Z2xvYmFsRGF0YSgp LCBwcm9wZXJ0eU5hbWUpKTsKKyAgICAgICAgICAgICAgICBpZiAoIXRoaXNPYmplY3QtPnB1dERp cmVjdEludGVybmFsPFB1dE1vZGVQdXQ+KAorICAgICAgICAgICAgICAgICAgICAgICAgZ2xvYmFs RGF0YSwgcHJvcGVydHlOYW1lLCB2YWx1ZSwgMCwgc2xvdCwgZ2V0Q2FsbGFibGVPYmplY3QodmFs dWUpKQorICAgICAgICAgICAgICAgICAgICAmJiBzbG90LmlzU3RyaWN0TW9kZSgpKQogICAgICAg ICAgICAgICAgICAgICB0aHJvd1R5cGVFcnJvcihleGVjLCBBU0NJSUxpdGVyYWwoU3RyaWN0TW9k ZVJlYWRvbmx5UHJvcGVydHlXcml0ZUVycm9yKSk7CiAgICAgICAgICAgICAgICAgcmV0dXJuOwog ICAgICAgICAgICAgfQogICAgICAgICB9CiAgICAgfQogCi0gICAgZm9yIChKU09iamVjdCogb2Jq ID0gdGhpc09iamVjdDsgOyBvYmogPSBhc09iamVjdChwcm90b3R5cGUpKSB7CisgICAgSlNPYmpl Y3QqIG9iajsKKyAgICBmb3IgKG9iaiA9IHRoaXNPYmplY3Q7IDsgb2JqID0gYXNPYmplY3QocHJv dG90eXBlKSkgewogICAgICAgICB1bnNpZ25lZCBhdHRyaWJ1dGVzOwogICAgICAgICBKU0NlbGwq IHNwZWNpZmljVmFsdWU7CiAgICAgICAgIFByb3BlcnR5T2Zmc2V0IG9mZnNldCA9IG9iai0+c3Ry dWN0dXJlKCktPmdldChnbG9iYWxEYXRhLCBwcm9wZXJ0eU5hbWUsIGF0dHJpYnV0ZXMsIHNwZWNp ZmljVmFsdWUpOwogICAgICAgICBpZiAoaXNWYWxpZE9mZnNldChvZmZzZXQpKSB7CiAgICAgICAg ICAgICBpZiAoYXR0cmlidXRlcyAmIFJlYWRPbmx5KSB7CisgICAgICAgICAgICAgICAgQVNTRVJU KHRoaXNPYmplY3QtPnN0cnVjdHVyZSgpLT5wcm90b3R5cGVDaGFpbk1heUludGVyY2VwdFN0b3Jl VG8oZXhlYy0+Z2xvYmFsRGF0YSgpLCBwcm9wZXJ0eU5hbWUpIHx8IG9iaiA9PSB0aGlzT2JqZWN0 KTsKICAgICAgICAgICAgICAgICBpZiAoc2xvdC5pc1N0cmljdE1vZGUoKSkKICAgICAgICAgICAg ICAgICAgICAgdGhyb3dFcnJvcihleGVjLCBjcmVhdGVUeXBlRXJyb3IoZXhlYywgQVNDSUlMaXRl cmFsKFN0cmljdE1vZGVSZWFkb25seVByb3BlcnR5V3JpdGVFcnJvcikpKTsKICAgICAgICAgICAg ICAgICByZXR1cm47CkBAIC0zODMsNiArMzg4LDggQEAgdm9pZCBKU09iamVjdDo6cHV0KEpTQ2Vs bCogY2VsbCwgRXhlY1N0YQogCiAgICAgICAgICAgICBKU1ZhbHVlIGdzID0gb2JqLT5nZXREaXJl Y3RPZmZzZXQob2Zmc2V0KTsKICAgICAgICAgICAgIGlmIChncy5pc0dldHRlclNldHRlcigpKSB7 CisgICAgICAgICAgICAgICAgQVNTRVJUKGF0dHJpYnV0ZXMgJiBBY2Nlc3Nvcik7CisgICAgICAg ICAgICAgICAgQVNTRVJUKHRoaXNPYmplY3QtPnN0cnVjdHVyZSgpLT5wcm90b3R5cGVDaGFpbk1h eUludGVyY2VwdFN0b3JlVG8oZXhlYy0+Z2xvYmFsRGF0YSgpLCBwcm9wZXJ0eU5hbWUpIHx8IG9i aiA9PSB0aGlzT2JqZWN0KTsKICAgICAgICAgICAgICAgICBKU09iamVjdCogc2V0dGVyRnVuYyA9 IGFzR2V0dGVyU2V0dGVyKGdzKS0+c2V0dGVyKCk7ICAgICAgICAKICAgICAgICAgICAgICAgICBp ZiAoIXNldHRlckZ1bmMpIHsKICAgICAgICAgICAgICAgICAgICAgaWYgKHNsb3QuaXNTdHJpY3RN b2RlKCkpCkBAIC0zOTgsNyArNDA1LDggQEAgdm9pZCBKU09iamVjdDo6cHV0KEpTQ2VsbCogY2Vs bCwgRXhlY1N0YQogICAgICAgICAgICAgICAgIC8vIElmIHRoaXMgaXMgV2ViQ29yZSdzIGdsb2Jh bCBvYmplY3QgdGhlbiB3ZSBuZWVkIHRvIHN1YnN0aXR1dGUgdGhlIHNoZWxsLgogICAgICAgICAg ICAgICAgIGNhbGwoZXhlYywgc2V0dGVyRnVuYywgY2FsbFR5cGUsIGNhbGxEYXRhLCB0aGlzT2Jq ZWN0LT5tZXRob2RUYWJsZSgpLT50b1RoaXNPYmplY3QodGhpc09iamVjdCwgZXhlYyksIGFyZ3Mp OwogICAgICAgICAgICAgICAgIHJldHVybjsKLSAgICAgICAgICAgIH0KKyAgICAgICAgICAgIH0g ZWxzZQorICAgICAgICAgICAgICAgIEFTU0VSVCghKGF0dHJpYnV0ZXMgJiBBY2Nlc3NvcikpOwog CiAgICAgICAgICAgICAvLyBJZiB0aGVyZSdzIGFuIGV4aXN0aW5nIHByb3BlcnR5IG9uIHRoZSBv YmplY3Qgb3Igb25lIG9mIGl0cyAKICAgICAgICAgICAgIC8vIHByb3RvdHlwZXMgaXQgc2hvdWxk IGJlIHJlcGxhY2VkLCBzbyBicmVhayBoZXJlLgpAQCAtNDEwLDYgKzQxOCw3IEBAIHZvaWQgSlNP YmplY3Q6OnB1dChKU0NlbGwqIGNlbGwsIEV4ZWNTdGEKICAgICAgICAgICAgIGJyZWFrOwogICAg IH0KICAgICAKKyAgICBBU1NFUlQoIXRoaXNPYmplY3QtPnN0cnVjdHVyZSgpLT5wcm90b3R5cGVD aGFpbk1heUludGVyY2VwdFN0b3JlVG8oZXhlYy0+Z2xvYmFsRGF0YSgpLCBwcm9wZXJ0eU5hbWUp IHx8IG9iaiA9PSB0aGlzT2JqZWN0KTsKICAgICBpZiAoIXRoaXNPYmplY3QtPnB1dERpcmVjdElu dGVybmFsPFB1dE1vZGVQdXQ+KGdsb2JhbERhdGEsIHByb3BlcnR5TmFtZSwgdmFsdWUsIDAsIHNs b3QsIGdldENhbGxhYmxlT2JqZWN0KHZhbHVlKSkgJiYgc2xvdC5pc1N0cmljdE1vZGUoKSkKICAg ICAgICAgdGhyb3dUeXBlRXJyb3IoZXhlYywgQVNDSUlMaXRlcmFsKFN0cmljdE1vZGVSZWFkb25s eVByb3BlcnR5V3JpdGVFcnJvcikpOwogICAgIHJldHVybjsKSW5kZXg6IFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9ydW50aW1lL1N0cnVjdHVyZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFT Y3JpcHRDb3JlL3J1bnRpbWUvU3RydWN0dXJlLmNwcAkocmV2aXNpb24gMTM0Njk1KQorKysgU291 cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvU3RydWN0dXJlLmNwcAkod29ya2luZyBjb3B5KQpA QCAtODYyLDYgKzg2MiwzMiBAQCB2b2lkIFN0cnVjdHVyZTo6dmlzaXRDaGlsZHJlbihKU0NlbGwq IGNlCiAgICAgdmlzaXRvci5hcHBlbmQoJnRoaXNPYmplY3QtPm1fb2JqZWN0VG9TdHJpbmdWYWx1 ZSk7CiB9CiAKK2Jvb2wgU3RydWN0dXJlOjpwcm90b3R5cGVDaGFpbk1heUludGVyY2VwdFN0b3Jl VG8oSlNHbG9iYWxEYXRhJiBnbG9iYWxEYXRhLCBQcm9wZXJ0eU5hbWUgcHJvcGVydHlOYW1lKQor eworICAgIHVuc2lnbmVkIGkgPSBwcm9wZXJ0eU5hbWUuYXNJbmRleCgpOworICAgIGlmIChpICE9 IFByb3BlcnR5TmFtZTo6Tm90QW5JbmRleCkKKyAgICAgICAgcmV0dXJuIGFueU9iamVjdEluQ2hh aW5NYXlJbnRlcmNlcHRJbmRleGVkQWNjZXNzZXMoKTsKKyAgICAKKyAgICBmb3IgKFN0cnVjdHVy ZSogY3VycmVudCA9IHRoaXM7IDspIHsKKyAgICAgICAgSlNWYWx1ZSBwcm90b3R5cGUgPSBjdXJy ZW50LT5zdG9yZWRQcm90b3R5cGUoKTsKKyAgICAgICAgaWYgKHByb3RvdHlwZS5pc051bGwoKSkK KyAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgCisgICAgICAgIGN1cnJlbnQgPSBw cm90b3R5cGUuYXNDZWxsKCktPnN0cnVjdHVyZSgpOworICAgICAgICAKKyAgICAgICAgdW5zaWdu ZWQgYXR0cmlidXRlczsKKyAgICAgICAgSlNDZWxsKiBzcGVjaWZpY1ZhbHVlOworICAgICAgICBQ cm9wZXJ0eU9mZnNldCBvZmZzZXQgPSBjdXJyZW50LT5nZXQoZ2xvYmFsRGF0YSwgcHJvcGVydHlO YW1lLCBhdHRyaWJ1dGVzLCBzcGVjaWZpY1ZhbHVlKTsKKyAgICAgICAgaWYgKCFKU0M6OmlzVmFs aWRPZmZzZXQob2Zmc2V0KSkKKyAgICAgICAgICAgIGNvbnRpbnVlOworICAgICAgICAKKyAgICAg ICAgaWYgKGF0dHJpYnV0ZXMgJiAoUmVhZE9ubHkgfCBBY2Nlc3NvcikpCisgICAgICAgICAgICBy ZXR1cm4gdHJ1ZTsKKyAgICAgICAgCisgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICB9Cit9CisK ICNpZiBET19QUk9QRVJUWU1BUF9DT05TVEVOQ1lfQ0hFQ0sKIAogdm9pZCBQcm9wZXJ0eVRhYmxl OjpjaGVja0NvbnNpc3RlbmN5KCkKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1l L1N0cnVjdHVyZS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1l L1N0cnVjdHVyZS5oCShyZXZpc2lvbiAxMzQ2OTUpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUv cnVudGltZS9TdHJ1Y3R1cmUuaAkod29ya2luZyBjb3B5KQpAQCAtMTY4LDcgKzE2OCwxMCBAQCBu YW1lc3BhY2UgSlNDIHsKICAgICAgICAgSlNWYWx1ZSBwcm90b3R5cGVGb3JMb29rdXAoQ29kZUJs b2NrKikgY29uc3Q7CiAgICAgICAgIFN0cnVjdHVyZUNoYWluKiBwcm90b3R5cGVDaGFpbihFeGVj U3RhdGUqKSBjb25zdDsKICAgICAgICAgc3RhdGljIHZvaWQgdmlzaXRDaGlsZHJlbihKU0NlbGwq LCBTbG90VmlzaXRvciYpOwotCisgICAgICAgIAorICAgICAgICAvLyBXaWxsIGp1c3QgdGhlIHBy b3RvdHlwZSBjaGFpbiBpbnRlcmNlcHQgdGhpcyBwcm9wZXJ0eSBhY2Nlc3M/CisgICAgICAgIGJv b2wgcHJvdG90eXBlQ2hhaW5NYXlJbnRlcmNlcHRTdG9yZVRvKEpTR2xvYmFsRGF0YSYsIFByb3Bl cnR5TmFtZSk7CisgICAgICAgIAogICAgICAgICBTdHJ1Y3R1cmUqIHByZXZpb3VzSUQoKSBjb25z dAogICAgICAgICB7CiAgICAgICAgICAgICBBU1NFUlQoc3RydWN0dXJlKCktPmNsYXNzSW5mbygp ID09ICZzX2luZm8pOwo=