65799 2011-08-05 17:19:22 -0700 String replace with the empty string means string removal 2011-08-05 19:18:15 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 barraclough webkit-unassigned webkit.review.bot oldest_to_newest 447489 0 barraclough 2011-08-05 17:19:22 -0700 Optimization for String.prototype.replace([RegExp], ""), this improves v8-regexp by ~3%. 447490 1 103133 barraclough 2011-08-05 17:22:11 -0700 Created attachment 103133 The patch 447496 2 webkit.review.bot 2011-08-05 17:26:32 -0700 Attachment 103133 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/StringPrototype.cpp:267: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/runtime/StringPrototype.cpp:271: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] Source/JavaScriptCore/runtime/StringPrototype.cpp:380: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 447500 3 103133 sam 2011-08-05 17:34:29 -0700 Comment on attachment 103133 The patch View in context: https://bugs.webkit.org/attachment.cgi?id=103133&action=review >> Source/JavaScriptCore/runtime/StringPrototype.cpp:271 >> + PassRefPtr<StringImpl> impl = StringImpl::tryCreateUninitialized(totalLength, buffer); > > Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] This is copy and pasted, but very wrong. Please us a RefPtr and fix the other spot. 447545 4 barraclough 2011-08-05 19:18:15 -0700 Fixed in r92540 103133 2011-08-05 17:22:11 -0700 2011-08-05 17:34:29 -0700 The patch 65799.patch text/plain 4390 barraclough SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gOTI1MjkpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAK KzIwMTEtMDgtMDUgIEdhdmluIEJhcnJhY2xvdWdoICA8YmFycmFjbG91Z2hAYXBwbGUuY29tPgor CisgICAgICAgIFN0cmluZyByZXBsYWNlIHdpdGggdGhlIGVtcHR5IHN0cmluZyBtZWFucyBzdHJp bmcgcmVtb3ZhbAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9NjU3OTkKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBPcHRpbWl6YXRpb24gZm9yIFN0cmluZy5wcm90b3R5cGUucmVwbGFjZShbUmVnRXhwXSwgIiIp LCB0aGlzIGltcHJvdmVzIHY4LXJlZ2V4cCBieSB+MyUuCisKKyAgICAgICAgKiBydW50aW1lL1N0 cmluZ1Byb3RvdHlwZS5jcHA6CisgICAgICAgIChKU0M6OmpzU3BsaWNlU3Vic3RyaW5ncyk6Cisg ICAgICAgIChKU0M6OnN0cmluZ1Byb3RvRnVuY1JlcGxhY2UpOgorCiAyMDExLTA4LTA1ICBNYXJr IFJvd2UgIDxtcm93ZUBhcHBsZS5jb20+CiAKICAgICAgICAgPGh0dHA6Ly93ZWJraXQub3JnL2Iv NjU3ODU+IFRocmVhZFJlc3RyaWN0aW9uVmVyaWZpZXIgbmVlZHMgYSBtb2RlIHdoZXJlIGFuIG9i amVjdApJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvU3RyaW5nUHJvdG90eXBl LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9TdHJpbmdQ cm90b3R5cGUuY3BwCShyZXZpc2lvbiA5MjQwNSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9y dW50aW1lL1N0cmluZ1Byb3RvdHlwZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTI0OCw2ICsyNDgs NDEgQEAgcHVibGljOgogICAgIGludCBsZW5ndGg7CiB9OwogCitzdGF0aWMgQUxXQVlTX0lOTElO RSBKU1ZhbHVlIGpzU3BsaWNlU3Vic3RyaW5ncyhFeGVjU3RhdGUqIGV4ZWMsIEpTU3RyaW5nKiBz b3VyY2VWYWwsIGNvbnN0IFVTdHJpbmcmIHNvdXJjZSwgY29uc3QgU3RyaW5nUmFuZ2UqIHN1YnN0 cmluZ1JhbmdlcywgaW50IHJhbmdlQ291bnQpCit7CisgICAgaWYgKHJhbmdlQ291bnQgPT0gMSkg eworICAgICAgICBpbnQgc291cmNlU2l6ZSA9IHNvdXJjZS5sZW5ndGgoKTsKKyAgICAgICAgaW50 IHBvc2l0aW9uID0gc3Vic3RyaW5nUmFuZ2VzWzBdLnBvc2l0aW9uOworICAgICAgICBpbnQgbGVu Z3RoID0gc3Vic3RyaW5nUmFuZ2VzWzBdLmxlbmd0aDsKKyAgICAgICAgaWYgKHBvc2l0aW9uIDw9 IDAgJiYgbGVuZ3RoID49IHNvdXJjZVNpemUpCisgICAgICAgICAgICByZXR1cm4gc291cmNlVmFs OworICAgICAgICAvLyBXZSBjb3VsZCBjYWxsIFVTdHJpbmc6OnN1YnN0ciwgYnV0IHRoaXMgd291 bGQgcmVzdWx0IGluIHJlZHVuZGFudCBjaGVja3MKKyAgICAgICAgcmV0dXJuIGpzU3RyaW5nKGV4 ZWMsIFN0cmluZ0ltcGw6OmNyZWF0ZShzb3VyY2UuaW1wbCgpLCBtYXgoMCwgcG9zaXRpb24pLCBt aW4oc291cmNlU2l6ZSwgbGVuZ3RoKSkpOworICAgIH0KKworICAgIGludCB0b3RhbExlbmd0aCA9 IDA7CisgICAgZm9yIChpbnQgaSA9IDA7IGkgPCByYW5nZUNvdW50OyBpKyspCisgICAgICAgIHRv dGFsTGVuZ3RoICs9IHN1YnN0cmluZ1Jhbmdlc1tpXS5sZW5ndGg7CisKKyAgICBpZiAodG90YWxM ZW5ndGggPT0gMCkKKyAgICAgICAgcmV0dXJuIGpzU3RyaW5nKGV4ZWMsICIiKTsKKworICAgIFVD aGFyKiBidWZmZXI7CisgICAgUGFzc1JlZlB0cjxTdHJpbmdJbXBsPiBpbXBsID0gU3RyaW5nSW1w bDo6dHJ5Q3JlYXRlVW5pbml0aWFsaXplZCh0b3RhbExlbmd0aCwgYnVmZmVyKTsKKyAgICBpZiAo IWltcGwpCisgICAgICAgIHJldHVybiB0aHJvd091dE9mTWVtb3J5RXJyb3IoZXhlYyk7CisKKyAg ICBpbnQgYnVmZmVyUG9zID0gMDsKKyAgICBmb3IgKGludCBpID0gMDsgaSA8IHJhbmdlQ291bnQ7 IGkrKykgeworICAgICAgICBpZiAoaW50IHNyY0xlbiA9IHN1YnN0cmluZ1Jhbmdlc1tpXS5sZW5n dGgpIHsKKyAgICAgICAgICAgIFN0cmluZ0ltcGw6OmNvcHlDaGFycyhidWZmZXIgKyBidWZmZXJQ b3MsIHNvdXJjZS5jaGFyYWN0ZXJzKCkgKyBzdWJzdHJpbmdSYW5nZXNbaV0ucG9zaXRpb24sIHNy Y0xlbik7CisgICAgICAgICAgICBidWZmZXJQb3MgKz0gc3JjTGVuOworICAgICAgICB9CisgICAg fQorCisgICAgcmV0dXJuIGpzU3RyaW5nKGV4ZWMsIGltcGwpOworfQorCiBzdGF0aWMgQUxXQVlT X0lOTElORSBKU1ZhbHVlIGpzU3BsaWNlU3Vic3RyaW5nc1dpdGhTZXBhcmF0b3JzKEV4ZWNTdGF0 ZSogZXhlYywgSlNTdHJpbmcqIHNvdXJjZVZhbCwgY29uc3QgVVN0cmluZyYgc291cmNlLCBjb25z dCBTdHJpbmdSYW5nZSogc3Vic3RyaW5nUmFuZ2VzLCBpbnQgcmFuZ2VDb3VudCwgY29uc3QgVVN0 cmluZyogc2VwYXJhdG9ycywgaW50IHNlcGFyYXRvckNvdW50KQogewogICAgIGlmIChyYW5nZUNv dW50ID09IDEgJiYgc2VwYXJhdG9yQ291bnQgPT0gMCkgewpAQCAtMzIwLDYgKzM1NSw0NCBAQCBF bmNvZGVkSlNWYWx1ZSBKU0NfSE9TVF9DQUxMIHN0cmluZ1Byb3RvCiAKICAgICAgICAgUmVnRXhw Q29uc3RydWN0b3IqIHJlZ0V4cENvbnN0cnVjdG9yID0gZXhlYy0+bGV4aWNhbEdsb2JhbE9iamVj dCgpLT5yZWdFeHBDb25zdHJ1Y3RvcigpOwogCisgICAgICAgIC8vIE9wdGltaXphdGlvbiBmb3Ig c3Vic3RyaW5nIHJlbW92YWwgKHJlcGxhY2Ugd2l0aCBlbXB0eSkuCisgICAgICAgIGlmIChnbG9i YWwgJiYgY2FsbFR5cGUgPT0gQ2FsbFR5cGVOb25lICYmICFyZXBsYWNlbWVudFN0cmluZy5sZW5n dGgoKSkgeworICAgICAgICAgICAgaW50IGxhc3RJbmRleCA9IDA7CisgICAgICAgICAgICB1bnNp Z25lZCBzdGFydFBvc2l0aW9uID0gMDsKKworICAgICAgICAgICAgVmVjdG9yPFN0cmluZ1Jhbmdl LCAxNj4gc291cmNlUmFuZ2VzOworCisgICAgICAgICAgICB3aGlsZSAodHJ1ZSkgeworICAgICAg ICAgICAgICAgIGludCBtYXRjaEluZGV4OworICAgICAgICAgICAgICAgIGludCBtYXRjaExlbiA9 IDA7CisgICAgICAgICAgICAgICAgaW50KiBvdmVjdG9yOworICAgICAgICAgICAgICAgIHJlZ0V4 cENvbnN0cnVjdG9yLT5wZXJmb3JtTWF0Y2goKmdsb2JhbERhdGEsIHJlZywgc291cmNlLCBzdGFy dFBvc2l0aW9uLCBtYXRjaEluZGV4LCBtYXRjaExlbiwgJm92ZWN0b3IpOworICAgICAgICAgICAg ICAgIGlmIChtYXRjaEluZGV4IDwgMCkKKyAgICAgICAgICAgICAgICAgICAgYnJlYWs7CisKKyAg ICAgICAgICAgICAgICBpZiAobGFzdEluZGV4IDwgbWF0Y2hJbmRleCkKKyAgICAgICAgICAgICAg ICAgICAgc291cmNlUmFuZ2VzLmFwcGVuZChTdHJpbmdSYW5nZShsYXN0SW5kZXgsIG1hdGNoSW5k ZXggLSBsYXN0SW5kZXgpKTsKKworICAgICAgICAgICAgICAgIGxhc3RJbmRleCA9IG1hdGNoSW5k ZXggKyBtYXRjaExlbjsKKyAgICAgICAgICAgICAgICBzdGFydFBvc2l0aW9uID0gbGFzdEluZGV4 OworCisgICAgICAgICAgICAgICAgLy8gc3BlY2lhbCBjYXNlIG9mIGVtcHR5IG1hdGNoCisgICAg ICAgICAgICAgICAgaWYgKG1hdGNoTGVuID09IDApIHsKKyAgICAgICAgICAgICAgICAgICAgc3Rh cnRQb3NpdGlvbisrOworICAgICAgICAgICAgICAgICAgICBpZiAoc3RhcnRQb3NpdGlvbiA+IHNv dXJjZUxlbikKKyAgICAgICAgICAgICAgICAgICAgICAgIGJyZWFrOworICAgICAgICAgICAgICAg IH0KKyAgICAgICAgICAgIH0KKworICAgICAgICAgICAgaWYgKCFsYXN0SW5kZXgpCisgICAgICAg ICAgICAgICAgcmV0dXJuIEpTVmFsdWU6OmVuY29kZShzb3VyY2VWYWwpOworCisgICAgICAgICAg ICBpZiAoc3RhdGljX2Nhc3Q8dW5zaWduZWQ+KGxhc3RJbmRleCkgPCBzb3VyY2VMZW4pCisgICAg ICAgICAgICAgICAgc291cmNlUmFuZ2VzLmFwcGVuZChTdHJpbmdSYW5nZShsYXN0SW5kZXgsIHNv dXJjZUxlbiAtIGxhc3RJbmRleCkpOworCisgICAgICAgICAgICByZXR1cm4gSlNWYWx1ZTo6ZW5j b2RlKGpzU3BsaWNlU3Vic3RyaW5ncyhleGVjLCBzb3VyY2VWYWwsIHNvdXJjZSwgc291cmNlUmFu Z2VzLmRhdGEoKSwgc291cmNlUmFuZ2VzLnNpemUoKSkpOworICAgICAgICB9CisKICAgICAgICAg aW50IGxhc3RJbmRleCA9IDA7CiAgICAgICAgIHVuc2lnbmVkIHN0YXJ0UG9zaXRpb24gPSAwOwog Cg==