41894 2010-07-08 13:07:49 -0700 Add a script that checks for unnecessary #includes in header files 2010-07-09 01:37:15 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 dumi dumi abarth eric krit oldest_to_newest 248474 0 dumi 2010-07-08 13:07:49 -0700 We should add a script that checks for unnecessary #includes in header files. Once the script is submitted, we can consider adding it to check-webkit-style, and/or use it to manually clean up header files, and/or add other scripts on top of it that automatically remove the unnecessary #includes. 248597 1 60974 dumi 2010-07-08 15:40:20 -0700 Created attachment 60974 patch This is the script I have. I'm sure it still needs lots of work, so I expect lots of comments. :) 248767 2 60974 darin 2010-07-08 23:40:39 -0700 Comment on attachment 60974 patch Great idea. I think we can land it as is, as long as it's not giving us any bad advice. No real need to carefully review it. Does this replace the existing find-extra-includes script already in the WebKitTools/Scripts directory, or does it have a different purpose? r=me on landing this, but please remove find-extra-includes if this replaces it, or don’t land this if find-extra-includes already does a better job. 248778 3 dumi 2010-07-08 23:55:38 -0700 > I think we can land it as is, as long as it's not giving us any bad advice. technically, it's possible to get some false positives. for example, if the header file has implementations that look like this: #include "A.h" class B { void doSomething() { getInstanceOfClassA()->foo(); } }; in this case, the script will say that it's not necessary to include A.h, which is not true. but i think these cases are quite rare (and maybe the implementation should be moved to the .cpp file?). also, we could probably add per-file whitelists, if this really becomes a problem. > Does this replace the existing find-extra-includes script already in the WebKitTools/Scripts directory, or does it have a different purpose? i'm looking at find-extra-includes, and if i understand the code correctly, there are 2 differences: 1. find-extra-includes works with any file; check-header-includes is supposed to work with header files only. 2. find-extra-includes reports only the includes that are not used at all; check-header-includes should also report the includes that could be replaced with forward declarations. > r=me on landing this, but please remove find-extra-includes if this replaces it, or don’t land this if find-extra-includes already does a better job. i'm going to land the script and use it to start cleaning up header files based on its output. if we need to improve it in any way, i'm happy to patch it as necessary. 248832 4 dumi 2010-07-09 01:37:15 -0700 landed: r62917. 60974 2010-07-08 15:40:20 -0700 2010-07-08 23:40:39 -0700 patch patch text/plain 5238 dumi SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDYyODU3KQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTIgQEAKKzIwMTAtMDctMDggIER1bWl0cnUgRGFuaWxpdWMg IDxkdW1pQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBBZGQgYSBzY3JpcHQgdG8gY2hlY2sgZm9yIHVubmVjZXNzYXJ5IGluY2x1 ZGVzIGluIGhlYWRlciBmaWxlcy4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTQxODk0CisKKyAgICAgICAgKiBTY3JpcHRzL2NoZWNrLWhlYWRlci1pbmNs dWRlczogQWRkZWQuCisKIDIwMTAtMDctMDggIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNlckBh cHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQW5kZXJzIENhcmxzc29uLgpJbmRleDog V2ViS2l0VG9vbHMvU2NyaXB0cy9jaGVjay1oZWFkZXItaW5jbHVkZXMKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g V2ViS2l0VG9vbHMvU2NyaXB0cy9jaGVjay1oZWFkZXItaW5jbHVkZXMJKHJldmlzaW9uIDApCisr KyBXZWJLaXRUb29scy9TY3JpcHRzL2NoZWNrLWhlYWRlci1pbmNsdWRlcwkocmV2aXNpb24gMCkK QEAgLTAsMCArMSwxMDYgQEAKKyMhL3Vzci9iaW4vcHl0aG9uCisjIENvcHlyaWdodCAoQykgMjAx MCBHb29nbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgorIworIyBSZWRpc3RyaWJ1dGlvbiBh bmQgdXNlIGluIHNvdXJjZSBhbmQgYmluYXJ5IGZvcm1zLCB3aXRoIG9yIHdpdGhvdXQKKyMgbW9k aWZpY2F0aW9uLCBhcmUgcGVybWl0dGVkIHByb3ZpZGVkIHRoYXQgdGhlIGZvbGxvd2luZyBjb25k aXRpb25zIGFyZQorIyBtZXQ6CisjCisjICAgICAqIFJlZGlzdHJpYnV0aW9ucyBvZiBzb3VyY2Ug Y29kZSBtdXN0IHJldGFpbiB0aGUgYWJvdmUgY29weXJpZ2h0CisjIG5vdGljZSwgdGhpcyBsaXN0 IG9mIGNvbmRpdGlvbnMgYW5kIHRoZSBmb2xsb3dpbmcgZGlzY2xhaW1lci4KKyMgICAgICogUmVk aXN0cmlidXRpb25zIGluIGJpbmFyeSBmb3JtIG11c3QgcmVwcm9kdWNlIHRoZSBhYm92ZQorIyBj b3B5cmlnaHQgbm90aWNlLCB0aGlzIGxpc3Qgb2YgY29uZGl0aW9ucyBhbmQgdGhlIGZvbGxvd2lu ZyBkaXNjbGFpbWVyCisjIGluIHRoZSBkb2N1bWVudGF0aW9uIGFuZC9vciBvdGhlciBtYXRlcmlh bHMgcHJvdmlkZWQgd2l0aCB0aGUKKyMgZGlzdHJpYnV0aW9uLgorIyAgICAgKiBOZWl0aGVyIHRo ZSBuYW1lIG9mIEdvb2dsZSBJbmMuIG5vciB0aGUgbmFtZXMgb2YgaXRzCisjIGNvbnRyaWJ1dG9y cyBtYXkgYmUgdXNlZCB0byBlbmRvcnNlIG9yIHByb21vdGUgcHJvZHVjdHMgZGVyaXZlZCBmcm9t CisjIHRoaXMgc29mdHdhcmUgd2l0aG91dCBzcGVjaWZpYyBwcmlvciB3cml0dGVuIHBlcm1pc3Np b24uCisjCisjIFRISVMgU09GVFdBUkUgSVMgUFJPVklERUQgQlkgVEhFIENPUFlSSUdIVCBIT0xE RVJTIEFORCBDT05UUklCVVRPUlMKKyMgIkFTIElTIiBBTkQgQU5ZIEVYUFJFU1MgT1IgSU1QTElF RCBXQVJSQU5USUVTLCBJTkNMVURJTkcsIEJVVCBOT1QKKyMgTElNSVRFRCBUTywgVEhFIElNUExJ RUQgV0FSUkFOVElFUyBPRiBNRVJDSEFOVEFCSUxJVFkgQU5EIEZJVE5FU1MgRk9SCisjIEEgUEFS VElDVUxBUiBQVVJQT1NFIEFSRSBESVNDTEFJTUVELiBJTiBOTyBFVkVOVCBTSEFMTCBUSEUgQ09Q WVJJR0hUCisjIE9XTkVSIE9SIENPTlRSSUJVVE9SUyBCRSBMSUFCTEUgRk9SIEFOWSBESVJFQ1Qs IElORElSRUNULCBJTkNJREVOVEFMLAorIyBTUEVDSUFMLCBFWEVNUExBUlksIE9SIENPTlNFUVVF TlRJQUwgREFNQUdFUyAoSU5DTFVESU5HLCBCVVQgTk9UCisjIExJTUlURUQgVE8sIFBST0NVUkVN RU5UIE9GIFNVQlNUSVRVVEUgR09PRFMgT1IgU0VSVklDRVM7IExPU1MgT0YgVVNFLAorIyBEQVRB LCBPUiBQUk9GSVRTOyBPUiBCVVNJTkVTUyBJTlRFUlJVUFRJT04pIEhPV0VWRVIgQ0FVU0VEIEFO RCBPTiBBTlkKKyMgVEhFT1JZIE9GIExJQUJJTElUWSwgV0hFVEhFUiBJTiBDT05UUkFDVCwgU1RS SUNUIExJQUJJTElUWSwgT1IgVE9SVAorIyAoSU5DTFVESU5HIE5FR0xJR0VOQ0UgT1IgT1RIRVJX SVNFKSBBUklTSU5HIElOIEFOWSBXQVkgT1VUIE9GIFRIRSBVU0UKKyMgT0YgVEhJUyBTT0ZUV0FS RSwgRVZFTiBJRiBBRFZJU0VEIE9GIFRIRSBQT1NTSUJJTElUWSBPRiBTVUNIIERBTUFHRS4KKyMK KyMgVGhpcyBzY3JpcHQgbG9va3MgZm9yIHVubmVjZXNzYXJ5ICNpbmNsdWRlcyBpbiB0aGUgZ2l2 ZW4gaGVhZGVyIGZpbGUsCisjIG9yIGluIGFsbCBoZWFkZXIgZmlsZXMgdGhhdCBjYW4gYmUgcmVj dXJzaXZlbHkgZm91bmQgaW4gdGhlIGdpdmVuIGRpcmVjdG9yeS4KKworaW1wb3J0IG9zCitpbXBv cnQgcmUKK2ltcG9ydCBzeXMKKworaGVhZGVyRmlsZSA9IHJlLmNvbXBpbGUocidcLmgkJykKK2lu Y2x1ZGVMaW5lID0gcmUuY29tcGlsZShyJ14jaW5jbHVkZSAoInw8d3RmLykoW0EtWl1cdyspLmhb PiJdJykKK25hbWVzcGFjZUxpbmUgPSByZS5jb21waWxlKHInXlxzKm5hbWVzcGFjZScpCitzbWFy dFBvaW50ZXJTdHJpbmcgPSAiKExpc3RSZWZQdHJ8T3duQXJyYXlQdHJ8T3duUHRyfFBhc3NPd25Q dHJ8UGFzc1JlZlB0cnxSZWZQdHIpPGNsYXNzTmFtZT4iCithbGxvd2VkSW5jbHVkZXMgPSBzZXQo WyJGb3J3YXJkIiwgIlBsYXRmb3JtU3RyaW5nIiwgIlN0cmluZ0hhc2giLCAiVGhyZWFkaW5nIl0p CisKKyMgQ2hlY2sgaWYgJ2xpbmUnIG5lZWRzIGFueSBjbGFzcyBpbiAndW5uZWNlc3NhcnlJbmNs dWRlcycgdG8gYmUgaW5jbHVkZWQgaW4gdGhlIGhlYWRlciBmaWxlLgorZGVmIGNoZWNrTGluZShs aW5lLCB1bm5lY2Vzc2FyeUluY2x1ZGVzKToKKyAgICB1bm5lY2Vzc2FyeUluY2x1ZGVzQ29weSA9 IHVubmVjZXNzYXJ5SW5jbHVkZXMuY29weSgpCisgICAgZm9yIGluY2x1ZGVkQ2xhc3MgaW4gdW5u ZWNlc3NhcnlJbmNsdWRlc0NvcHk6CisgICAgICAgIGlmIHJlLnNlYXJjaChpbmNsdWRlZENsYXNz LCBsaW5lKToKKyAgICAgICAgICAgIGlmIHJlLnNlYXJjaChpbmNsdWRlZENsYXNzICsgIlwqIiwg bGluZSk6CisgICAgICAgICAgICAgICAgY29udGludWUKKworICAgICAgICAgICAgc21hcnRQb2lu dGVyQ2xhc3NTdHJpbmcgPSBzbWFydFBvaW50ZXJTdHJpbmcucmVwbGFjZSgiY2xhc3NOYW1lIiwg aW5jbHVkZWRDbGFzcykKKyAgICAgICAgICAgIGlmIHJlLnNlYXJjaChzbWFydFBvaW50ZXJDbGFz c1N0cmluZywgbGluZSk6CisgICAgICAgICAgICAgICAgY29udGludWUKKworICAgICAgICAgICAg dW5uZWNlc3NhcnlJbmNsdWRlcy5yZW1vdmUoaW5jbHVkZWRDbGFzcykKKworCisjIExvb2sgZm9y IHVubmVjZXNzYXJ5IGluY2x1ZGVzIGluIHRoZSBnaXZlbiBmaWxlCitkZWYgY2hlY2tGaWxlKGZp bGVOYW1lKToKKyAgICBpZiBoZWFkZXJGaWxlLnNlYXJjaChmaWxlTmFtZSk6CisgICAgICAgIGYg PSBvcGVuKGZpbGVOYW1lLCAiciIpCisgICAgICAgIHVubmVjZXNzYXJ5SW5jbHVkZXMgPSBzZXQo KQorICAgICAgICBuYW1lc3BhY2VGb3VuZCA9IEZhbHNlCisgICAgICAgIGZvciBsaW5lIGluIGY6 CisgICAgICAgICAgICBtYXRjaCA9IGluY2x1ZGVMaW5lLm1hdGNoKGxpbmUpCisgICAgICAgICAg ICBpZiBtYXRjaDoKKyAgICAgICAgICAgICAgICBjbGFzc05hbWUgPSBtYXRjaC5ncm91cCgyKQor ICAgICAgICAgICAgICAgIGlmIGNsYXNzTmFtZSA9PSAiUGxhdGZvcm1TdHJpbmciOgorICAgICAg ICAgICAgICAgICAgICB1bm5lY2Vzc2FyeUluY2x1ZGVzLmFkZCgiU3RyaW5nIikKKyAgICAgICAg ICAgICAgICBlbGlmIGNsYXNzTmFtZSBub3QgaW4gYWxsb3dlZEluY2x1ZGVzOgorICAgICAgICAg ICAgICAgICAgICB1bm5lY2Vzc2FyeUluY2x1ZGVzLmFkZChjbGFzc05hbWUpCisgICAgICAgICAg ICBlbGlmIG5hbWVzcGFjZUxpbmUuc2VhcmNoKGxpbmUpOgorICAgICAgICAgICAgICAgIG5hbWVz cGFjZUZvdW5kID0gVHJ1ZQorICAgICAgICAgICAgZWxpZiBuYW1lc3BhY2VGb3VuZDoKKyAgICAg ICAgICAgICAgICBjaGVja0xpbmUobGluZSwgdW5uZWNlc3NhcnlJbmNsdWRlcykKKworICAgICAg ICBpZiBsZW4odW5uZWNlc3NhcnlJbmNsdWRlcykgPiAwOgorICAgICAgICAgICAgcHJpbnQgZmls ZU5hbWUKKyAgICAgICAgICAgIGZvciBpbmNsdWRlZENsYXNzIGluIHVubmVjZXNzYXJ5SW5jbHVk ZXM6CisgICAgICAgICAgICAgICAgcHJpbnQgaW5jbHVkZWRDbGFzcworCisKKyMgSWYgJ3BhdGgn IGlzIGEgZmlsZSwgY2hlY2sgaWYgaXQgaGFzIHVubmVjZXNzYXJ5IGluY2x1ZGVzLgorIyBJZiAn cGF0aCcgaXMgYSBkaXJlY3RvcnksIHRyYXZlcnNlIGl0IHJlY3Vyc2l2ZWx5IGFuZCBsb29rCisj IGZvciB1bm5lY2Vzc2FyeSBpbmNsdWRlcyBpbiBhbGwgaGVhZGVyIGZpbGVzLgorZGVmIGNoZWNr UGF0aChwYXRoKToKKyAgICBpZiBvcy5wYXRoLmlzZmlsZShwYXRoKToKKyAgICAgICAgY2hlY2tG aWxlKHBhdGgpCisgICAgZWxpZiBvcy5wYXRoLmlzZGlyKHBhdGgpOgorICAgICAgICBkaXJMaXN0 ID0gb3MubGlzdGRpcihwYXRoKQorICAgICAgICBmb3IgZGlyTmFtZSBpbiBkaXJMaXN0OgorICAg ICAgICAgICAgY2hlY2tQYXRoKHBhdGggKyAiLyIgKyBkaXJOYW1lKQorICAgIGVsc2U6CisgICAg ICAgIHByaW50IHBhdGgsICJpcyBub3QgYSBmaWxlIG9yIGRpcmVjdG9yeS4iCisKKworZGVmIG1h aW4oKToKKyAgICBpZiBsZW4oc3lzLmFyZ3YpIDw9IDE6CisgICAgICAgIHByaW50ICJVc2FnZToi LCBzeXMuYXJndlswXSwgIjxwYXRoPiIKKyAgICAgICAgcmV0dXJuIC0xCisKKyAgICBjaGVja1Bh dGgoc3lzLmFyZ3ZbMV0pCisKKworaWYgX19uYW1lX18gPT0gIl9fbWFpbl9fIjoKKyAgICBtYWlu KCkKClByb3BlcnR5IGNoYW5nZXMgb246IFdlYktpdFRvb2xzL1NjcmlwdHMvY2hlY2staGVhZGVy LWluY2x1ZGVzCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX18KQWRkZWQ6IHN2bjpleGVjdXRhYmxlCiAgICsgKgoK