163575 2016-10-17 17:41:40 -0700 check-webkit-style: fix false-positive warnings about using #pragma once header guard 2016-10-19 12:53:27 -0700 1 1 1 Unclassified WebKit Tools / Tests Other All All RESOLVED FIXED P2 Normal --- 1 megan_gardner webkit-unassigned beidson commit-queue glenn joepeck lforschler simon.fraser oldest_to_newest 1241278 0 megan_gardner 2016-10-17 17:41:40 -0700 The style guide gave me false positives for errors in TestRunnerWKWebView.h and WKWebViewPrivate.h about the #pragma once header guards. Not needed because of obj-c #import. 1241697 1 joepeck 2016-10-18 16:57:06 -0700 Ooo good point... I wonder if we can tell if such a file will be ObjC/ObjC++. We could employ heuristics like "if there is an #import". 1241938 2 beidson 2016-10-19 09:16:45 -0700 I think the simple solution is to detect *if* there is a header guard, *then* it has to be pragma once. Headers without guards are clearly okay (as if they weren't okay then the builds would be failing) 1241990 3 joepeck 2016-10-19 11:04:30 -0700 > Headers without guards are clearly okay (as if they weren't okay then the > builds would be failing) It would be nice to detect and warn when a header is missing header guards that _should_ have header guards. A build may succeed if its missing, but later when someone else double includes the header it could fail. That said, your suggestion should catch the majority of cases. Patch coming. 1241991 4 292084 joepeck 2016-10-19 11:04:45 -0700 Created attachment 292084 [PATCH] Proposed Fix 1242038 5 beidson 2016-10-19 12:28:27 -0700 (In reply to comment #3) > > Headers without guards are clearly okay (as if they weren't okay then the > > builds would be failing) > > It would be nice to detect and warn when a header is missing header guards > that _should_ have header guards. A build may succeed if its missing, but > later when someone else double includes the header it could fail. Yah, I guess that was the unspoken subtext of what I meant. I also am not sure what the actual compiler error is when you double include a header - Whether it's unintelligible random errors, or if the compiler explicitly points it out. In the later case, I think it'd be fine to just ignore it until an error pops up. 1242039 6 292084 beidson 2016-10-19 12:29:16 -0700 Comment on attachment 292084 [PATCH] Proposed Fix lgtm 1242047 7 292084 commit-queue 2016-10-19 12:53:23 -0700 Comment on attachment 292084 [PATCH] Proposed Fix Clearing flags on attachment: 292084 Committed r207559: <http://trac.webkit.org/changeset/207559> 1242048 8 commit-queue 2016-10-19 12:53:27 -0700 All reviewed patches have been landed. Closing bug. 292084 2016-10-19 11:04:45 -0700 2016-10-19 12:53:23 -0700 [PATCH] Proposed Fix pragma-once-1.patch text/plain 3131 joepeck ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBhNGFm ZTZkLi4zNjg3NDliIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjMgQEAKKzIwMTYtMTAtMTkgIEpvc2VwaCBQZWNvcmFybyAgPHBl Y29yYXJvQGFwcGxlLmNvbT4KKworICAgICAgICBjaGVjay13ZWJraXQtc3R5bGU6IGZpeCBmYWxz ZS1wb3NpdGl2ZSB3YXJuaW5ncyBhYm91dCB1c2luZyAjcHJhZ21hIG9uY2UgaGVhZGVyIGd1YXJk CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjM1NzUK KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGVyZSB3 ZXJlIHNvbWUgZmFsc2UgcG9zaXRpdmUgd2FybmluZ3MgYWJvdXQgaGVhZGVyIGd1YXJkcworICAg ICAgICBmb3IgT2JqQyBoZWFkZXJzIHRoYXQgZG9uJ3QgbmVlZCBndWFyZHMuIFRoaXMgY2hhbmdl cyB0aGUKKyAgICAgICAgc3R5bGUgY2hlY2tlciB0byBubyBsb25nZXIgd2FybiBpZiB0aGVyZSBp cyBubyAjcHJhZ21hIG9uY2UuCisgICAgICAgIEl0IG9ubHkgd2FybnMgaWYgdGhlcmUgYXJlICNp Zm5kZWYgdGhhdCBzaG91bGQgYmUgI3ByYWdtYSBvbmNlLgorCisgICAgICAgICogU2NyaXB0cy93 ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHAucHk6CisgICAgICAgIChjaGVja19mb3JfaGVhZGVy X2d1YXJkKToKKyAgICAgICAgQ2hhbmdlIHdoZW4gd2Ugb3V0cHV0IGFuIGVycm9yLgorCisgICAg ICAgICogU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHk6Cisg ICAgICAgIChDcHBTdHlsZVRlc3QudGVzdF9idWlsZF9oZWFkZXJfZ3VhcmQpOgorICAgICAgICBV cGRhdGUgdGVzdHMuCisKIDIwMTYtMTAtMTcgIEFuZHkgRXN0ZXMgIDxhZXN0ZXNAYXBwbGUuY29t PgogCiAgICAgICAgIENyYXNoIGluIEFTQ0lJQ2FzZUluc2Vuc2l0aXZlSGFzaDo6aGFzaCgpIHdo ZW4gYSByZXNwb25zZSBoYXMgYSBudWxsIE1JTUUgdHlwZQpkaWZmIC0tZ2l0IGEvVG9vbHMvU2Ny aXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHAucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtp dHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQppbmRleCBjOWI0MTJhLi5mMjAzMmEyIDEwMDY0NAot LS0gYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQorKysgYi9U b29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcC5weQpAQCAtODk5LDggKzg5 OSw4IEBAIGRlZiBjaGVja19mb3JfY29weXJpZ2h0KGxpbmVzLCBlcnJvcik6CiBkZWYgY2hlY2tf Zm9yX2hlYWRlcl9ndWFyZChsaW5lcywgZXJyb3IpOgogICAgICIiIkNoZWNrcyB0aGF0IHRoZSBm aWxlIGNvbnRhaW5zIGEgaGVhZGVyIGd1YXJkLgogCi0gICAgTG9ncyBhbiBlcnJvciBpZiBubyAj cHJhZ21hIG9uY2UgaGVhZGVyIGd1YXJkIGlzIHByZXNlbnQKLSAgICBvZiBpZiB0aGVyZSB3YXMg YW4gI2lmbmRlZiBndWFyZCB0aGF0IHdhcyBtb2RpZmllZC4KKyAgICBMb2dzIGFuIGVycm9yIGlm IHRoZXJlIHdhcyBhbiAjaWZuZGVmIGd1YXJkIGluIGEgaGVhZGVyCisgICAgdGhhdCBzaG91bGQg YmUgYSAjcHJhZ21hIG9uY2UgZ3VhcmQuCiAKICAgICBBcmdzOgogICAgICAgbGluZXM6IEFuIGFy cmF5IG9mIHN0cmluZ3MsIGVhY2ggcmVwcmVzZW50aW5nIGEgbGluZSBvZiB0aGUgZmlsZS4KQEAg LTkyMSw5ICs5MjEsNiBAQCBkZWYgY2hlY2tfZm9yX2hlYWRlcl9ndWFyZChsaW5lcywgZXJyb3Ip OgogICAgICAgICAgICAgICAgICAgICAnVXNlICNwcmFnbWEgb25jZSBpbnN0ZWFkIG9mICNpZm5k ZWYgZm9yIGhlYWRlciBndWFyZC4nKQogICAgICAgICAgICAgICAgIHJldHVybgogCi0gICAgZXJy b3IoMCwgJ2J1aWxkL2hlYWRlcl9ndWFyZCcsIDUsCi0gICAgICAgICdVc2UgI3ByYWdtYSBvbmNl IGhlYWRlciBndWFyZC4nKQotCiAKIGRlZiBjaGVja19mb3JfdW5pY29kZV9yZXBsYWNlbWVudF9j aGFyYWN0ZXJzKGxpbmVzLCBlcnJvcik6CiAgICAgIiIiTG9ncyBhbiBlcnJvciBmb3IgZWFjaCBs aW5lIGNvbnRhaW5pbmcgVW5pY29kZSByZXBsYWNlbWVudCBjaGFyYWN0ZXJzLgpkaWZmIC0tZ2l0 IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9zdHlsZS9jaGVja2Vycy9jcHBfdW5pdHRlc3QucHkg Yi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3N0eWxlL2NoZWNrZXJzL2NwcF91bml0dGVzdC5weQpp bmRleCBkNWRkYjc0Li44YzNhMDM3IDEwMDY0NAotLS0gYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5 L3N0eWxlL2NoZWNrZXJzL2NwcF91bml0dGVzdC5weQorKysgYi9Ub29scy9TY3JpcHRzL3dlYmtp dHB5L3N0eWxlL2NoZWNrZXJzL2NwcF91bml0dGVzdC5weQpAQCAtMjQyNywxNiArMjQyNywxNCBA QCBjbGFzcyBDcHBTdHlsZVRlc3QoQ3BwU3R5bGVUZXN0QmFzZSk6CiAgICAgZGVmIHRlc3RfYnVp bGRfaGVhZGVyX2d1YXJkKHNlbGYpOgogICAgICAgICBydWxlcyA9ICgnLScsICcrYnVpbGQvaGVh ZGVyX2d1YXJkJykKIAotICAgICAgICAjIE5vIGhlYWRlciBndWFyZC4KLSAgICAgICAgc2VsZi5h c3NlcnRfaGVhZGVyX2d1YXJkKCcnLAotICAgICAgICAgICAgJ1VzZSAjcHJhZ21hIG9uY2UgaGVh ZGVyIGd1YXJkLicKLSAgICAgICAgICAgICcgIFtidWlsZC9oZWFkZXJfZ3VhcmRdIFs1XScpCi0K ICAgICAgICAgIyBPbGQgaGVhZGVyIGd1YXJkLgogICAgICAgICBzZWxmLmFzc2VydF9oZWFkZXJf Z3VhcmQoJyNpZm5kZWYgRm9vX2gnLAogICAgICAgICAgICAgJ1VzZSAjcHJhZ21hIG9uY2UgaW5z dGVhZCBvZiAjaWZuZGVmIGZvciBoZWFkZXIgZ3VhcmQuJwogICAgICAgICAgICAgJyAgW2J1aWxk L2hlYWRlcl9ndWFyZF0gWzVdJykKIAorICAgICAgICAjIE5vIGhlYWRlciBndWFyZC4gT2theSwg c2luY2UgdGhpcyBjb3VsZCBiZSBhbiBPYmpDIGhlYWRlci4KKyAgICAgICAgc2VsZi5hc3NlcnRf aGVhZGVyX2d1YXJkKCcnLCAnJykKKwogICAgICAgICAjIFZhbGlkIGhlYWRlciBndWFyZC4KICAg ICAgICAgc2VsZi5hc3NlcnRfaGVhZGVyX2d1YXJkKCcjcHJhZ21hIG9uY2UnLCAnJykKIAo=