32991 2009-12-28 12:33:27 -0800 [Qt] check-webkit-style generates false report on "#include *.moc" 2009-12-29 15:06:48 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC Linux RESOLVED FIXED Qt P3 Normal --- 1 cshu cshu commit-queue eric hamaji levin webkit.review.bot oldest_to_newest 175283 0 cshu 2009-12-28 12:33:27 -0800 check-webkit-style generated the following error report on patch 45564 in bug 32614. Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/wtf/qt/ThreadingQt.cpp:293: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 1 I think it's a false alarm. Lines like "#include ThreadingQt.moc" in a Qt cpp file should not follow the general include style. 175285 1 45568 cshu 2009-12-28 12:43:32 -0800 Created attachment 45568 fix patch Include files ending with ".moc" were accidentally categorized as PRIMARY_HEADER. 175289 2 webkit.review.bot 2009-12-28 12:47:10 -0800 style-queue ran check-webkit-style on attachment 45568 without any errors. 175462 3 45568 eric 2009-12-29 00:06:09 -0800 Comment on attachment 45568 fix patch This requires a unittest. I'm not familiar enough with this code to know if this is the right approach. 175541 4 cshu 2009-12-29 06:53:51 -0800 (In reply to comment #3) > (From update of attachment 45568 [details]) > This requires a unittest. I'm not familiar enough with this code to know if > this is the right approach. The line I have changed is to check the primary header by comparing the base file name. For example, if the cpp file is test.cpp, test.h will be the primary header. However, for Qt, a line such as "#include test.moc" may show up at the end of the file. Its base name is also "test" but we should exclude this from primary header. 175619 5 45612 cshu 2009-12-29 10:15:31 -0800 Created attachment 45612 alternative fix This is an alternative fix. It simply changes the order of processing headers. 175621 6 webkit.review.bot 2009-12-29 10:19:19 -0800 style-queue ran check-webkit-style on attachment 45612 without any errors. 175667 7 45612 abarth 2009-12-29 11:47:21 -0800 Comment on attachment 45612 alternative fix I'm not a cpp_lint expert, but this second patch looks like a good approach. However, we still need a test. cpp_lint has extensive unit tests. It should be straightforward to modify one to test this new behavior. 175691 8 45620 cshu 2009-12-29 13:19:24 -0800 Created attachment 45620 added unit tests 2 new unit tests added for Qt moc headers. 175692 9 webkit.review.bot 2009-12-29 13:21:17 -0800 style-queue ran check-webkit-style on attachment 45620 without any errors. 175701 10 45620 abarth 2009-12-29 13:58:35 -0800 Comment on attachment 45620 added unit tests LGTM. A cpp_lint expert might want to double check. Thanks for the test. 175722 11 45620 commit-queue 2009-12-29 15:06:40 -0800 Comment on attachment 45620 added unit tests Clearing flags on attachment: 45620 Committed r52636: <http://trac.webkit.org/changeset/52636> 175723 12 commit-queue 2009-12-29 15:06:48 -0800 All reviewed patches have been landed. Closing bug. 45568 2009-12-28 12:43:32 -0800 2009-12-29 10:15:31 -0800 fix patch bug32991_1.patch text/plain 1402 cshu SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDUyNjAyKQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMDktMTItMjggIFNodSBDaGFuZyAgPENoYW5n LlNodUBub2tpYS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgW1F0XSBJbmNsdWRlIGZpbGVzIGVuZGluZyB3aXRoICIubW9jIiBzaG91bGQgbm90 IGJlIGNvbnNpZGVyZWQKKyAgICAgICAgYXMgX1BSSU1BUllfSEVBREVSLgorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzI5OTEKKworICAgICAgICAqIFNj cmlwdHMvbW9kdWxlcy9jcHBfc3R5bGUucHk6CisKIDIwMDktMTItMjggIExhc3psbyBHb21ib3Mg IDxsYXN6bG8uMS5nb21ib3NAbm9raWEuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEFkYW0g QmFydGguCkluZGV4OiBXZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvY3BwX3N0eWxlLnB5Cj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFdlYktpdFRvb2xzL1NjcmlwdHMvbW9kdWxlcy9jcHBfc3R5bGUucHkJKHJl dmlzaW9uIDUyNjAxKQorKysgV2ViS2l0VG9vbHMvU2NyaXB0cy9tb2R1bGVzL2NwcF9zdHlsZS5w eQkod29ya2luZyBjb3B5KQpAQCAtMjIyNyw3ICsyMjI3LDcgQEAgZGVmIF9jbGFzc2lmeV9pbmNs dWRlKGZpbGVuYW1lLCBpbmNsdWRlLAogICAgICMgSWYgd2UgYWxyZWFkeSBlbmNvdW50ZXJlZCBh IHByaW1hcnkgaGVhZGVyLCBwZXJmb3JtIGEgc3RyaWN0IGNvbXBhcmlzb24uCiAgICAgIyBJbiBj YXNlIHRoZSB0d28gZmlsZW5hbWUgYmFzZXMgYXJlIHRoZSBzYW1lIHRoZW4gdGhlIGFib3ZlIGxl bmllbnQgY2hlY2sKICAgICAjIHByb2JhYmx5IHdhcyBhIGZhbHNlIHBvc2l0aXZlLgotICAgIGVs aWYgaW5jbHVkZV9zdGF0ZS52aXNpdGVkX3ByaW1hcnlfc2VjdGlvbigpIGFuZCB0YXJnZXRfYmFz ZSA9PSBpbmNsdWRlX2Jhc2U6CisgICAgZWxpZiBpbmNsdWRlX3N0YXRlLnZpc2l0ZWRfcHJpbWFy eV9zZWN0aW9uKCkgYW5kIHRhcmdldF9iYXNlID09IGluY2x1ZGVfYmFzZSBhbmQgbm90IGluY2x1 ZGUuZW5kc3dpdGgoJy5tb2MnKToKICAgICAgICAgcmV0dXJuIF9QUklNQVJZX0hFQURFUgogCiAg ICAgIyBRdCdzIG1vYyBmaWxlcyBkbyBub3QgZm9sbG93IHRoZSBuYW1pbmcgYW5kIG9yZGVyaW5n IHJ1bGVzLCBzbyB0aGV5IHNob3VsZCBiZSBza2lwcGVkCg== 45612 2009-12-29 10:15:31 -0800 2009-12-29 13:19:24 -0800 alternative fix bug32991_2.patch text/plain 1921 cshu SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDUyNjIwKQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMDktMTItMjkgIFNodSBDaGFuZyAgPENoYW5n LlNodUBub2tpYS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgW1F0XSBNb3ZlIGxvZ2ljIHRoYXQgaGFuZGxlcyBRdCBoZWFkZXJzIGJlZm9yZSBw cmltYXJ5IGhlYWRlcnMKKyAgICAgICAgc28gdGhhdCBRdCBoZWFkZXJzIHdvbid0IGJlIHRyZWF0 ZWQgYXMgcHJpbWFyeSBoZWFkZXJzIGJ5IG1pc3Rha2UuCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMjk5MQorCisgICAgICAgICogU2NyaXB0cy9tb2R1 bGVzL2NwcF9zdHlsZS5weToKKwogMjAwOS0xMi0yOSAgR3VzdGF2byBOb3JvbmhhIFNpbHZhICA8 Z3VzdGF2by5ub3JvbmhhQGNvbGxhYm9yYS5jby51az4KIAogICAgICAgICBSZXZpZXdlZCBieSBI b2xnZXIgRnJleXRoZXIuCkluZGV4OiBXZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvY3BwX3N0 eWxlLnB5Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFdlYktpdFRvb2xzL1NjcmlwdHMvbW9kdWxlcy9jcHBfc3R5 bGUucHkJKHJldmlzaW9uIDUyNjIwKQorKysgV2ViS2l0VG9vbHMvU2NyaXB0cy9tb2R1bGVzL2Nw cF9zdHlsZS5weQkod29ya2luZyBjb3B5KQpAQCAtMjIxNiw2ICsyMjE2LDEzIEBAIGRlZiBfY2xh c3NpZnlfaW5jbHVkZShmaWxlbmFtZSwgaW5jbHVkZSwKICAgICBpZiBmaWxlbmFtZS5lbmRzd2l0 aCgnLmgnKSBhbmQgZmlsZW5hbWUgIT0gaW5jbHVkZToKICAgICAgICAgcmV0dXJuIF9PVEhFUl9I RUFERVI7CiAKKyAgICAjIFF0J3MgbW9jIGZpbGVzIGRvIG5vdCBmb2xsb3cgdGhlIG5hbWluZyBh bmQgb3JkZXJpbmcgcnVsZXMsIHNvIHRoZXkgc2hvdWxkIGJlIHNraXBwZWQKKyAgICBpZiBpbmNs dWRlLnN0YXJ0c3dpdGgoJ21vY18nKSBhbmQgaW5jbHVkZS5lbmRzd2l0aCgnLmNwcCcpOgorICAg ICAgICByZXR1cm4gX01PQ19IRUFERVIKKworICAgIGlmIGluY2x1ZGUuZW5kc3dpdGgoJy5tb2Mn KToKKyAgICAgICAgcmV0dXJuIF9NT0NfSEVBREVSCisKICAgICAjIElmIHRoZSB0YXJnZXQgZmls ZSBiYXNlbmFtZSBzdGFydHMgd2l0aCB0aGUgaW5jbHVkZSB3ZSdyZSBjaGVja2luZwogICAgICMg dGhlbiB3ZSBjb25zaWRlciBpdCB0aGUgcHJpbWFyeSBoZWFkZXIuCiAgICAgdGFyZ2V0X2Jhc2Ug PSBGaWxlSW5mbyhmaWxlbmFtZSkuYmFzZV9uYW1lKCkKQEAgLTIyMzAsMTMgKzIyMzcsNiBAQCBk ZWYgX2NsYXNzaWZ5X2luY2x1ZGUoZmlsZW5hbWUsIGluY2x1ZGUsCiAgICAgZWxpZiBpbmNsdWRl X3N0YXRlLnZpc2l0ZWRfcHJpbWFyeV9zZWN0aW9uKCkgYW5kIHRhcmdldF9iYXNlID09IGluY2x1 ZGVfYmFzZToKICAgICAgICAgcmV0dXJuIF9QUklNQVJZX0hFQURFUgogCi0gICAgIyBRdCdzIG1v YyBmaWxlcyBkbyBub3QgZm9sbG93IHRoZSBuYW1pbmcgYW5kIG9yZGVyaW5nIHJ1bGVzLCBzbyB0 aGV5IHNob3VsZCBiZSBza2lwcGVkCi0gICAgaWYgaW5jbHVkZS5zdGFydHN3aXRoKCdtb2NfJykg YW5kIGluY2x1ZGUuZW5kc3dpdGgoJy5jcHAnKToKLSAgICAgICAgcmV0dXJuIF9NT0NfSEVBREVS Ci0KLSAgICBpZiBpbmNsdWRlLmVuZHN3aXRoKCcubW9jJyk6Ci0gICAgICAgIHJldHVybiBfTU9D X0hFQURFUgotCiAgICAgcmV0dXJuIF9PVEhFUl9IRUFERVIKIAogCg== 45620 2009-12-29 13:19:24 -0800 2009-12-29 15:06:40 -0800 added unit tests bug32991_3.patch text/plain 3116 cshu SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDUyNjMyKQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMDktMTItMjkgIENoYW5nIFNodSAgPENoYW5n LlNodUBub2tpYS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgW1F0XSBNb3ZlIGxvZ2ljIHRoYXQgaGFuZGxlcyBRdCBoZWFkZXJzIGJlZm9yZSBw cmltYXJ5IGhlYWRlcnMKKyAgICAgICAgc28gdGhhdCBRdCBoZWFkZXJzIHdvbid0IGJlIHRyZWF0 ZWQgYXMgcHJpbWFyeSBoZWFkZXJzIGJ5IG1pc3Rha2UuCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMjk5MQorCisgICAgICAgICogU2NyaXB0cy9tb2R1 bGVzL2NwcF9zdHlsZS5weToKKyAgICAgICAgKiBTY3JpcHRzL21vZHVsZXMvY3BwX3N0eWxlX3Vu aXR0ZXN0LnB5OgorCiAyMDA5LTEyLTI5ICBFcmljIFNlaWRlbCAgPGVyaWNAd2Via2l0Lm9yZz4K IAogICAgICAgICBSZXZpZXdlZCBieSBBZGFtIEJhcnRoLgpJbmRleDogV2ViS2l0VG9vbHMvU2Ny aXB0cy9tb2R1bGVzL2NwcF9zdHlsZS5weQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9TY3Jp cHRzL21vZHVsZXMvY3BwX3N0eWxlLnB5CShyZXZpc2lvbiA1MjYzMCkKKysrIFdlYktpdFRvb2xz L1NjcmlwdHMvbW9kdWxlcy9jcHBfc3R5bGUucHkJKHdvcmtpbmcgY29weSkKQEAgLTIyMTYsNiAr MjIxNiwxMyBAQCBkZWYgX2NsYXNzaWZ5X2luY2x1ZGUoZmlsZW5hbWUsIGluY2x1ZGUsCiAgICAg aWYgZmlsZW5hbWUuZW5kc3dpdGgoJy5oJykgYW5kIGZpbGVuYW1lICE9IGluY2x1ZGU6CiAgICAg ICAgIHJldHVybiBfT1RIRVJfSEVBREVSOwogCisgICAgIyBRdCdzIG1vYyBmaWxlcyBkbyBub3Qg Zm9sbG93IHRoZSBuYW1pbmcgYW5kIG9yZGVyaW5nIHJ1bGVzLCBzbyB0aGV5IHNob3VsZCBiZSBz a2lwcGVkCisgICAgaWYgaW5jbHVkZS5zdGFydHN3aXRoKCdtb2NfJykgYW5kIGluY2x1ZGUuZW5k c3dpdGgoJy5jcHAnKToKKyAgICAgICAgcmV0dXJuIF9NT0NfSEVBREVSCisKKyAgICBpZiBpbmNs dWRlLmVuZHN3aXRoKCcubW9jJyk6CisgICAgICAgIHJldHVybiBfTU9DX0hFQURFUgorCiAgICAg IyBJZiB0aGUgdGFyZ2V0IGZpbGUgYmFzZW5hbWUgc3RhcnRzIHdpdGggdGhlIGluY2x1ZGUgd2Un cmUgY2hlY2tpbmcKICAgICAjIHRoZW4gd2UgY29uc2lkZXIgaXQgdGhlIHByaW1hcnkgaGVhZGVy LgogICAgIHRhcmdldF9iYXNlID0gRmlsZUluZm8oZmlsZW5hbWUpLmJhc2VfbmFtZSgpCkBAIC0y MjMwLDEzICsyMjM3LDYgQEAgZGVmIF9jbGFzc2lmeV9pbmNsdWRlKGZpbGVuYW1lLCBpbmNsdWRl LAogICAgIGVsaWYgaW5jbHVkZV9zdGF0ZS52aXNpdGVkX3ByaW1hcnlfc2VjdGlvbigpIGFuZCB0 YXJnZXRfYmFzZSA9PSBpbmNsdWRlX2Jhc2U6CiAgICAgICAgIHJldHVybiBfUFJJTUFSWV9IRUFE RVIKIAotICAgICMgUXQncyBtb2MgZmlsZXMgZG8gbm90IGZvbGxvdyB0aGUgbmFtaW5nIGFuZCBv cmRlcmluZyBydWxlcywgc28gdGhleSBzaG91bGQgYmUgc2tpcHBlZAotICAgIGlmIGluY2x1ZGUu c3RhcnRzd2l0aCgnbW9jXycpIGFuZCBpbmNsdWRlLmVuZHN3aXRoKCcuY3BwJyk6Ci0gICAgICAg IHJldHVybiBfTU9DX0hFQURFUgotCi0gICAgaWYgaW5jbHVkZS5lbmRzd2l0aCgnLm1vYycpOgot ICAgICAgICByZXR1cm4gX01PQ19IRUFERVIKLQogICAgIHJldHVybiBfT1RIRVJfSEVBREVSCiAK IApJbmRleDogV2ViS2l0VG9vbHMvU2NyaXB0cy9tb2R1bGVzL2NwcF9zdHlsZV91bml0dGVzdC5w eQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvY3BwX3N0eWxlX3Vu aXR0ZXN0LnB5CShyZXZpc2lvbiA1MjYzMCkKKysrIFdlYktpdFRvb2xzL1NjcmlwdHMvbW9kdWxl cy9jcHBfc3R5bGVfdW5pdHRlc3QucHkJKHdvcmtpbmcgY29weSkKQEAgLTIyMjEsNiArMjIyMSwx NCBAQCBjbGFzcyBPcmRlck9mSW5jbHVkZXNUZXN0KENwcFN0eWxlVGVzdEJhCiAgICAgICAgICAg ICAgICAgICAgICAgICAgY2xhc3NpZnlfaW5jbHVkZSgnUHJlZml4Rm9vQ3VzdG9tLmNwcCcsCiAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAnRm9vLmgnLAogICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgRmFsc2UsIGluY2x1ZGVfc3RhdGUp KQorICAgICAgICBzZWxmLmFzc2VydEVxdWFsKGNwcF9zdHlsZS5fTU9DX0hFQURFUiwKKyAgICAg ICAgICAgICAgICAgICAgICAgICBjbGFzc2lmeV9pbmNsdWRlKCdmb28uY3BwJywKKyAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICdmb28ubW9jJywKKyAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIEZhbHNlLCBpbmNsdWRlX3N0YXRlKSkKKyAg ICAgICAgc2VsZi5hc3NlcnRFcXVhbChjcHBfc3R5bGUuX01PQ19IRUFERVIsCisgICAgICAgICAg ICAgICAgICAgICAgICAgY2xhc3NpZnlfaW5jbHVkZSgnZm9vLmNwcCcsCisgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAnbW9jX2Zvby5jcHAnLAorICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgRmFsc2UsIGluY2x1ZGVfc3RhdGUpKQogICAg ICAgICAjIFRyaWNreSBleGFtcGxlIHdoZXJlIGJvdGggaW5jbHVkZXMgbWlnaHQgYmUgY2xhc3Np ZmllZCBhcyBwcmltYXJ5LgogICAgICAgICBzZWxmLmFzc2VydF9sYW5ndWFnZV9ydWxlc19jaGVj aygnU2Nyb2xsYmFyVGhlbWVXaW5jZS5jcHAnLAogICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAnI2luY2x1ZGUgImNvbmZpZy5oIlxuJwo=