Bug 175652 - check-webkit-style shouldn't check spacing in #elif preprocessor lines
Summary: check-webkit-style shouldn't check spacing in #elif preprocessor lines
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-16 19:09 PDT by Yoshiaki Jitsukawa
Modified: 2017-08-17 19:38 PDT (History)
7 users (show)

See Also:

Attachments
Patch (1.85 KB, patch)
2017-08-16 19:16 PDT, Yoshiaki Jitsukawa 		no flags Details | Formatted Diff | Diff
Patch (2.56 KB, patch)
2017-08-17 12:54 PDT, Yoshiaki Jitsukawa 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yoshiaki Jitsukawa 2017-08-16 19:09:36 PDT 
check-webkit-style generates false positive [whitespace/operators] errors around path delimiter '/' for __has_include() predicate.
Comment 1 Yoshiaki Jitsukawa 2017-08-16 19:16:28 PDT 
Created attachment 318317 [details]
Patch
Comment 2 Yoshiaki Jitsukawa 2017-08-16 19:28:05 PDT 
e.g. ./platform/spi/mac/AVFoundationSPI.h:#if __has_include(<AVFoundation/AVSampleBufferRenderSynchronizer.h>)
Comment 3 Alex Christensen 2017-08-17 09:35:11 PDT 
I'm pretty sure check-webkit-style has tests.
Comment 4 Yoshiaki Jitsukawa 2017-08-17 12:26:52 PDT 
(In reply to Alex Christensen from comment #3)
> I'm pretty sure check-webkit-style has tests.

Yes
https://trac.webkit.org/browser/webkit/trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py#L2035
Comment 5 Yoshiaki Jitsukawa 2017-08-17 12:28:40 PDT 
(In reply to Yoshiaki Jitsukawa from comment #4)
> (In reply to Alex Christensen from comment #3)
> > I'm pretty sure check-webkit-style has tests.
> 
> Yes
> https://trac.webkit.org/browser/webkit/trunk/Tools/Scripts/webkitpy/style/
> checkers/cpp_unittest.py#L2035

Ah, the test passes because of "#if" so I needed to change this "#if" to something else.
Comment 6 Yoshiaki Jitsukawa 2017-08-17 12:54:05 PDT 
Created attachment 318404 [details]
Patch
Comment 7 Yoshiaki Jitsukawa 2017-08-17 12:57:56 PDT 
Originally I hit this issue with "#elif __has_include.." code so now I think
this should be handled by skipping spacing checks for "#elif".
Comment 8 Yoshiaki Jitsukawa 2017-08-17 13:00:04 PDT 
Related bug: Bug 125534 - check-webkit-style shouldn't check spacing in #if preprocessor lines
Comment 9 Yoshiaki Jitsukawa 2017-08-17 15:52:08 PDT 
Hi Alex, would you review my patch again if you are available?
Comment 10 WebKit Commit Bot 2017-08-17 19:17:10 PDT 
Comment on attachment 318404 [details]
Patch

Clearing flags on attachment: 318404

Committed r220896: <http://trac.webkit.org/changeset/220896>
Comment 11 WebKit Commit Bot 2017-08-17 19:17:11 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2017-08-17 19:18:48 PDT 
<rdar://problem/33955505>
Comment 13 Yoshiaki Jitsukawa 2017-08-17 19:38:09 PDT 
Thank you for your reviewing, Alex!