[webkit-reviews] review granted: [Bug 206480] check-webkit-style: Improve header guard checks : [Attachment 388178] Patch v3

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 206480: check-webkit-style: Improve header guard checks
https://bugs.webkit.org/show_bug.cgi?id=206480

Attachment 388178: Patch v3

https://bugs.webkit.org/attachment.cgi?id=388178&action=review




--- Comment #11 from Darin Adler <darin at apple.com> ---
Comment on attachment 388178
  --> https://bugs.webkit.org/attachment.cgi?id=388178
Patch v3

View in context: https://bugs.webkit.org/attachment.cgi?id=388178&action=review

>>>>> Tools/Scripts/webkitpy/style/checkers/cpp.py:965
>>>>> + 	   has_objc_keywords = True
>>>> 
>>>> A header with an Objective-C keyword is not necessarily an
Objective-C-only header. There could be an #ifdef __OBJC__ guarding the
Objective-C part.
>>> 
>>> Yep, see the `has_objc_check` check three lines above, and the logic for
returning early two lines below.
>>> 
>>> I felt Comment #6 was a pretty good indication of a low false-positive
rate.
>> 
>> For example, looking at Source/WebCore/platform/cocoa/SystemVersion.h (which
was flagged as missing a "#pragma once" statement in Comment #6) without
looking at where it's included, you might guess that it's an Objective-C++
header, but maybe it's relying on an `OBJC_CLASS NSString;` definition from
another header in UnifiedSources.
>> 
>> Since it's currently ambiguous, I'd probably add an "@class NSString;"
statement to the header to make it obvious that it's only used by
Objective-C[++] sources.
> 
> Here's an example of a true positive from Comment #6 with an __OBJC__ check
where the "#pragma once" statement is missing:
> 
> Source/WebCore/platform/network/mac/WebCoreURLResponse.h

I see. Had missed the other checks.