48242 2010-10-25 08:42:14 -0700 check-webkit-style: false positive reported for #if macro 2010-12-03 00:33:20 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 ddkilzer webkit-unassigned cjerdonek commit-queue eric hamaji levin ojan oldest_to_newest 298870 0 ddkilzer 2010-10-25 08:42:14 -0700 Attachment 71742 patch on Bug 46096, Comment #8 had a false-positive on an #if macro definition: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/assembler/MacroAssemblerARM.cpp:59: More than one command on the same line in if [whitespace/parens] [4] Total errors found: 1 in 2 files I believe the check-webkit-style script though this was a standard if() statement, not an #if macro. 316545 1 75440 levin 2010-12-02 17:20:14 -0800 Created attachment 75440 Patch 316547 2 75440 eric 2010-12-02 17:24:39 -0800 Comment on attachment 75440 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75440&action=review Otherwise seems fine. > WebKitTools/Scripts/webkitpy/style/checkers/cpp.py:1461 > + in_preprocessor_directive = match(r'\s*#', line) Seems you want a ^ in that regexp to lock it to the front of the string. char* my_invalid_style = "You're #1\n"'; would trigger that regexp even though it shouldn't. 316589 3 levin 2010-12-02 18:28:22 -0800 (In reply to comment #2) > (From update of attachment 75440 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=75440&action=review > > Otherwise seems fine. > > > WebKitTools/Scripts/webkitpy/style/checkers/cpp.py:1461 > > + in_preprocessor_directive = match(r'\s*#', line) > > Seems you want a ^ in that regexp to lock it to the front of the string. > > char* my_invalid_style = "You're #1\n"'; would trigger that regexp even though it shouldn't. Good call. fwiw, re.match has an implicit ^ (http://docs.python.org/library/re.html#re.match). re.search would need the ^ I wish there was only re.search (why have another function just to mean "^" + regex_given?) but match exists, so I guess it makes sense to use it. 316593 4 levin 2010-12-02 18:32:14 -0800 (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 75440 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=75440&action=review > > > > Otherwise seems fine. > > > > > WebKitTools/Scripts/webkitpy/style/checkers/cpp.py:1461 > > > + in_preprocessor_directive = match(r'\s*#', line) > > > > Seems you want a ^ in that regexp to lock it to the front of the string. > > > > char* my_invalid_style = "You're #1\n"'; would trigger that regexp even though it shouldn't. > Good call. > fwiw, re.match has an implicit ^ (http://docs.python.org/library/re.html#re.match). > re.search would need the ^ > > I wish there was only re.search (why have another function just to mean "^" + regex_given?) but match exists, so I guess it makes sense to use it. But I'll add test for this though. 316609 5 75451 levin 2010-12-02 19:04:47 -0800 Created attachment 75451 Patch 316615 6 75451 hamaji 2010-12-02 19:20:46 -0800 Comment on attachment 75451 Patch looks good 316707 7 75451 commit-queue 2010-12-03 00:33:14 -0800 Comment on attachment 75451 Patch Clearing flags on attachment: 75451 Committed r73248: <http://trac.webkit.org/changeset/73248> 316708 8 commit-queue 2010-12-03 00:33:20 -0800 All reviewed patches have been landed. Closing bug. 75440 2010-12-02 17:20:14 -0800 2010-12-02 19:04:43 -0800 Patch bug-48242-20101202172013.patch text/plain 2622 levin ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCA2YzAwMzc4NzcyYzU4ODMzNzY5NGY0MjA0NDc4NTBkMjczNDExNGY1Li4zNDY3NTgz ZDY4YTgyNTI5Y2NiYTYxMTY5MTY4ODcyMDQ0NzNkYzkwIDEwMDY0NAotLS0gYS9XZWJLaXRUb29s cy9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAK KzIwMTAtMTItMDIgIERhdmlkIExldmluICA8bGV2aW5AY2hyb21pdW0ub3JnPgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIGNoZWNrLXdlYmtpdC1zdHls ZTogZmFsc2UgcG9zaXRpdmUgcmVwb3J0ZWQgZm9yICNpZiBtYWNybworICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDgyNDIKKworICAgICAgICAqIFNjcmlw dHMvd2Via2l0cHkvc3R5bGUvY2hlY2tlcnMvY3BwLnB5OiBBZGp1c3RlZCBjaGVjayB0byBhdm9p ZAorICAgICAgICAgIGFsbCBwcmVwcm9jZXNzb3IgY29tbWFuZHMuCisgICAgICAgICogU2NyaXB0 cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHk6IEFkZGVkIHRlc3QuCisK IDIwMTAtMTItMDIgIEJyZW50IEZ1bGdoYW0gIDxiZnVsZ2hhbUB3ZWJraXQub3JnPgogCiAgICAg ICAgIFVucmV2aWV3ZWQgYnVpbGQgZml4IGFmdGVyIHZjcHJvaiB1cGRhdGVzLgpkaWZmIC0tZ2l0 IGEvV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHAucHkgYi9X ZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQppbmRleCA1 OTBiYmE5MmVkMGIwMWM4YTQ5MDc2NDJiYTNmNGYxOWU5NmMyNTRlLi41ODBlYjU4NDM2MWQzMjI3 MDcxZmQxMGYyNjk5MTYyNGYyNjcwMzk3IDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9TY3JpcHRz L3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQorKysgYi9XZWJLaXRUb29scy9TY3JpcHRz L3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQpAQCAtMTQ1OCw4ICsxNDU4LDggQEAgZGVm IGNoZWNrX3NwYWNpbmcoZmlsZV9leHRlbnNpb24sIGNsZWFuX2xpbmVzLCBsaW5lX251bWJlciwg ZXJyb3IpOgogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgJ0V4dHJhIHNwYWNlIGJlZm9y ZSApIGluICVzJyAlIHN0YXRlbWVudCkKIAogICAgICAgICAgICAgIyBEbyBub3QgY2hlY2sgZm9y IG1vcmUgdGhhbiBvbmUgY29tbWFuZCBpbiBtYWNyb3MKLSAgICAgICAgICAgIGluX21hY3JvID0g bWF0Y2gocidccyojZGVmaW5lJywgbGluZSkKLSAgICAgICAgICAgIGlmIG5vdCBpbl9tYWNybyBh bmQgbm90IG1hdGNoKHInKChccyp7XHMqfT8pfChccyo7PykpXHMqXFw/JCcsIHJlc3QpOgorICAg ICAgICAgICAgaW5fcHJlcHJvY2Vzc29yX2RpcmVjdGl2ZSA9IG1hdGNoKHInXHMqIycsIGxpbmUp CisgICAgICAgICAgICBpZiBub3QgaW5fcHJlcHJvY2Vzc29yX2RpcmVjdGl2ZSBhbmQgbm90IG1h dGNoKHInKChccyp7XHMqfT8pfChccyo7PykpXHMqXFw/JCcsIHJlc3QpOgogICAgICAgICAgICAg ICAgIGVycm9yKGxpbmVfbnVtYmVyLCAnd2hpdGVzcGFjZS9wYXJlbnMnLCA0LAogICAgICAgICAg ICAgICAgICAgICAgICdNb3JlIHRoYW4gb25lIGNvbW1hbmQgb24gdGhlIHNhbWUgbGluZSBpbiAl cycgJSBzdGF0ZW1lbnQpCiAKZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0 cHkvc3R5bGUvY2hlY2tlcnMvY3BwX3VuaXR0ZXN0LnB5IGIvV2ViS2l0VG9vbHMvU2NyaXB0cy93 ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHkKaW5kZXggMTNiMDUzY2JmNDk1 NGQ4MzE4ZDcyYzNhMTc5MTIzMjhjMjI0OTVkZS4uZmU0NTQxOGZiZDYyYTllNGYyYjkyOWI0NzM0 YWY3Nzk1NjJjMGQ0YyAxMDA2NDQKLS0tIGEvV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRweS9z dHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHkKKysrIGIvV2ViS2l0VG9vbHMvU2NyaXB0cy93 ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHkKQEAgLTMxNTIsNiArMzE1Miwx MCBAQCBjbGFzcyBXZWJLaXRTdHlsZVRlc3QoQ3BwU3R5bGVUZXN0QmFzZSk6CiAgICAgICAgIHNl bGYuYXNzZXJ0X211bHRpX2xpbmVfbGludCgKICAgICAgICAgICAgICcgICAgaWYgKGNvbmRpdGlv bikgZG9JdCgpO1xuJywKICAgICAgICAgICAgICdNb3JlIHRoYW4gb25lIGNvbW1hbmQgb24gdGhl IHNhbWUgbGluZSBpbiBpZiAgW3doaXRlc3BhY2UvcGFyZW5zXSBbNF0nKQorICAgICAgICAjIEln bm9yZSBwcmVwcm9jZXNzb3IgaWYncy4KKyAgICAgICAgc2VsZi5hc3NlcnRfbXVsdGlfbGluZV9s aW50KAorICAgICAgICAgICAgJyAgICAjaWYgKGNvbmRpdGlvbikgfHwgKGNvbmRpdGlvbjIpXG4n LAorICAgICAgICAgICAgJycpCiAKICAgICAgICAgIyAyLiBBbiBlbHNlIHN0YXRlbWVudCBzaG91 bGQgZ28gb24gdGhlIHNhbWUgbGluZSBhcyBhIHByZWNlZGluZwogICAgICAgICAjICAgY2xvc2Ug YnJhY2UgaWYgb25lIGlzIHByZXNlbnQsIGVsc2UgaXQgc2hvdWxkIGxpbmUgdXAgd2l0aCB0aGUK 75451 2010-12-02 19:04:47 -0800 2010-12-03 00:33:14 -0800 Patch bug-48242-20101202190446.patch text/plain 2852 levin ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCA2YzAwMzc4NzcyYzU4ODMzNzY5NGY0MjA0NDc4NTBkMjczNDExNGY1Li4zNDY3NTgz ZDY4YTgyNTI5Y2NiYTYxMTY5MTY4ODcyMDQ0NzNkYzkwIDEwMDY0NAotLS0gYS9XZWJLaXRUb29s cy9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAK KzIwMTAtMTItMDIgIERhdmlkIExldmluICA8bGV2aW5AY2hyb21pdW0ub3JnPgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIGNoZWNrLXdlYmtpdC1zdHls ZTogZmFsc2UgcG9zaXRpdmUgcmVwb3J0ZWQgZm9yICNpZiBtYWNybworICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDgyNDIKKworICAgICAgICAqIFNjcmlw dHMvd2Via2l0cHkvc3R5bGUvY2hlY2tlcnMvY3BwLnB5OiBBZGp1c3RlZCBjaGVjayB0byBhdm9p ZAorICAgICAgICAgIGFsbCBwcmVwcm9jZXNzb3IgY29tbWFuZHMuCisgICAgICAgICogU2NyaXB0 cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHk6IEFkZGVkIHRlc3QuCisK IDIwMTAtMTItMDIgIEJyZW50IEZ1bGdoYW0gIDxiZnVsZ2hhbUB3ZWJraXQub3JnPgogCiAgICAg ICAgIFVucmV2aWV3ZWQgYnVpbGQgZml4IGFmdGVyIHZjcHJvaiB1cGRhdGVzLgpkaWZmIC0tZ2l0 IGEvV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHAucHkgYi9X ZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQppbmRleCA1 OTBiYmE5MmVkMGIwMWM4YTQ5MDc2NDJiYTNmNGYxOWU5NmMyNTRlLi41ODBlYjU4NDM2MWQzMjI3 MDcxZmQxMGYyNjk5MTYyNGYyNjcwMzk3IDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9TY3JpcHRz L3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQorKysgYi9XZWJLaXRUb29scy9TY3JpcHRz L3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQpAQCAtMTQ1OCw4ICsxNDU4LDggQEAgZGVm IGNoZWNrX3NwYWNpbmcoZmlsZV9leHRlbnNpb24sIGNsZWFuX2xpbmVzLCBsaW5lX251bWJlciwg ZXJyb3IpOgogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgJ0V4dHJhIHNwYWNlIGJlZm9y ZSApIGluICVzJyAlIHN0YXRlbWVudCkKIAogICAgICAgICAgICAgIyBEbyBub3QgY2hlY2sgZm9y IG1vcmUgdGhhbiBvbmUgY29tbWFuZCBpbiBtYWNyb3MKLSAgICAgICAgICAgIGluX21hY3JvID0g bWF0Y2gocidccyojZGVmaW5lJywgbGluZSkKLSAgICAgICAgICAgIGlmIG5vdCBpbl9tYWNybyBh bmQgbm90IG1hdGNoKHInKChccyp7XHMqfT8pfChccyo7PykpXHMqXFw/JCcsIHJlc3QpOgorICAg ICAgICAgICAgaW5fcHJlcHJvY2Vzc29yX2RpcmVjdGl2ZSA9IG1hdGNoKHInXHMqIycsIGxpbmUp CisgICAgICAgICAgICBpZiBub3QgaW5fcHJlcHJvY2Vzc29yX2RpcmVjdGl2ZSBhbmQgbm90IG1h dGNoKHInKChccyp7XHMqfT8pfChccyo7PykpXHMqXFw/JCcsIHJlc3QpOgogICAgICAgICAgICAg ICAgIGVycm9yKGxpbmVfbnVtYmVyLCAnd2hpdGVzcGFjZS9wYXJlbnMnLCA0LAogICAgICAgICAg ICAgICAgICAgICAgICdNb3JlIHRoYW4gb25lIGNvbW1hbmQgb24gdGhlIHNhbWUgbGluZSBpbiAl cycgJSBzdGF0ZW1lbnQpCiAKZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0 cHkvc3R5bGUvY2hlY2tlcnMvY3BwX3VuaXR0ZXN0LnB5IGIvV2ViS2l0VG9vbHMvU2NyaXB0cy93 ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHkKaW5kZXggMTNiMDUzY2JmNDk1 NGQ4MzE4ZDcyYzNhMTc5MTIzMjhjMjI0OTVkZS4uNTNmYjE1ZTMzYjQwZGFlMTAzNzZjMDVjYTkz MTU5Yzc1YjQ0OTc3YSAxMDA2NDQKLS0tIGEvV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRweS9z dHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHkKKysrIGIvV2ViS2l0VG9vbHMvU2NyaXB0cy93 ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHkKQEAgLTMxNTIsNiArMzE1Miwx NCBAQCBjbGFzcyBXZWJLaXRTdHlsZVRlc3QoQ3BwU3R5bGVUZXN0QmFzZSk6CiAgICAgICAgIHNl bGYuYXNzZXJ0X211bHRpX2xpbmVfbGludCgKICAgICAgICAgICAgICcgICAgaWYgKGNvbmRpdGlv bikgZG9JdCgpO1xuJywKICAgICAgICAgICAgICdNb3JlIHRoYW4gb25lIGNvbW1hbmQgb24gdGhl IHNhbWUgbGluZSBpbiBpZiAgW3doaXRlc3BhY2UvcGFyZW5zXSBbNF0nKQorICAgICAgICAjIEVu c3VyZSB0aGF0IGhhdmluZyBhICMgaW4gdGhlIGxpbmUgZG9lc24ndCBoaWRlIHRoZSBlcnJvci4K KyAgICAgICAgc2VsZi5hc3NlcnRfbXVsdGlfbGluZV9saW50KAorICAgICAgICAgICAgJyAgICB4 Kys7IGNoYXIgYVtdID0gIiMiOycsCisgICAgICAgICAgICAnTW9yZSB0aGFuIG9uZSBjb21tYW5k IG9uIHRoZSBzYW1lIGxpbmUgIFt3aGl0ZXNwYWNlL25ld2xpbmVdIFs0XScpCisgICAgICAgICMg SWdub3JlIHByZXByb2Nlc3NvciBpZidzLgorICAgICAgICBzZWxmLmFzc2VydF9tdWx0aV9saW5l X2xpbnQoCisgICAgICAgICAgICAnICAgICNpZiAoY29uZGl0aW9uKSB8fCAoY29uZGl0aW9uMilc bicsCisgICAgICAgICAgICAnJykKIAogICAgICAgICAjIDIuIEFuIGVsc2Ugc3RhdGVtZW50IHNo b3VsZCBnbyBvbiB0aGUgc2FtZSBsaW5lIGFzIGEgcHJlY2VkaW5nCiAgICAgICAgICMgICBjbG9z ZSBicmFjZSBpZiBvbmUgaXMgcHJlc2VudCwgZWxzZSBpdCBzaG91bGQgbGluZSB1cCB3aXRoIHRo ZQo=