Bug 163575

Summary:

check-webkit-style: fix false-positive warnings about using #pragma once header guard

Product:

WebKit

Reporter:

Megan Gardner <megan_gardner>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

beidson, commit-queue, glenn, joepeck, lforschler, simon.fraser

Priority:

Version:

Other

Hardware:

All

OS:

All

Attachments:

Description	Flags
[PATCH] Proposed Fix	none

Description Megan Gardner 2016-10-17 17:41:40 PDT

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.

Comment 1 Joseph Pecoraro 2016-10-18 16:57:06 PDT

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".

Comment 2 Brady Eidson 2016-10-19 09:16:45 PDT

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)

Comment 3 Joseph Pecoraro 2016-10-19 11:04:30 PDT

> 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.

Comment 4 Joseph Pecoraro 2016-10-19 11:04:45 PDT

Created attachment 292084 [details]
[PATCH] Proposed Fix

Comment 5 Brady Eidson 2016-10-19 12:28:27 PDT

(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.

Comment 6 Brady Eidson 2016-10-19 12:29:16 PDT

Comment on attachment 292084 [details]
[PATCH] Proposed Fix

lgtm

Comment 7 WebKit Commit Bot 2016-10-19 12:53:23 PDT

Comment on attachment 292084 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 292084

Committed r207559: <http://trac.webkit.org/changeset/207559>

Comment 8 WebKit Commit Bot 2016-10-19 12:53:27 PDT

All reviewed patches have been landed.  Closing bug.