Bug 108745

Summary: WTF shouldn't use a script build phase to detect the presence of headers when the compiler can do it for us
Product: WebKit Reporter: Mark Rowe (bdash) <mrowe>
Component: Web Template FrameworkAssignee: Mark Rowe (bdash) <mrowe>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, ojan.autocc, philn, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=153729
Attachments:
Description Flags
Patch v1
eflews.bot: commit-queue-
Patch v2 none

Description Mark Rowe (bdash) 2013-02-01 23:44:48 PST
WTF has a build phase to detect the presence of certain headers at build time. This is unnecessary now that the Mac port always builds with Clang, which has the __has_include directive that can detect headers for us.
Comment 1 Mark Rowe (bdash) 2013-02-01 23:53:38 PST
Created attachment 186216 [details]
Patch v1
Comment 2 WebKit Review Bot 2013-02-01 23:56:05 PST
Attachment 186216 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/DerivedSources.make', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xcodeproj/project.pbxproj', u'Source/WTF/wtf/FastMalloc.cpp']" exit_code: 1
Source/WTF/wtf/FastMalloc.cpp:451:  Missing spaces around /  [whitespace/operators] [3]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Mark Rowe (bdash) 2013-02-02 00:18:06 PST
Comment on attachment 186216 [details]
Patch v1

Clearing flags since this appears to not build with GCC.
Comment 4 EFL EWS Bot 2013-02-02 00:20:42 PST
Comment on attachment 186216 [details]
Patch v1

Attachment 186216 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16343541
Comment 5 Mark Rowe (bdash) 2013-02-02 00:51:39 PST
Created attachment 186220 [details]
Patch v2

Split the detection of __has_include out from the use of it so that compilers that don't support it don't choke.
Comment 6 WebKit Review Bot 2013-02-02 00:58:41 PST
Attachment 186220 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/DerivedSources.make', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xcodeproj/project.pbxproj', u'Source/WTF/wtf/FastMalloc.cpp']" exit_code: 1
Source/WTF/wtf/FastMalloc.cpp:452:  Missing spaces around /  [whitespace/operators] [3]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Alexey Proskuryakov 2013-02-02 11:47:19 PST
Comment on attachment 186220 [details]
Patch v2

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

> Source/WTF/wtf/FastMalloc.cpp:451
> +#ifdef __has_include

Would it make sense to have a WTF macro for this (like HAVE(HAS_INCLUDE))?
Comment 8 WebKit Review Bot 2013-02-02 12:27:27 PST
Comment on attachment 186220 [details]
Patch v2

Clearing flags on attachment: 186220

Committed r141700: <http://trac.webkit.org/changeset/141700>
Comment 9 WebKit Review Bot 2013-02-02 12:27:30 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Mark Rowe (bdash) 2013-02-02 13:07:10 PST
(In reply to comment #7)
> (From update of attachment 186220 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186220&action=review
> 
> > Source/WTF/wtf/FastMalloc.cpp:451
> > +#ifdef __has_include
> 
> Would it make sense to have a WTF macro for this (like HAVE(HAS_INCLUDE))?

I considered doing something like COMPILER_SUPPORTS(HEADER_DETECTION). In order to make it worth it you'd want to wrap the actual use of __has_include in a macro like HAS_INCLUDE  to support compilers with different names for the functionality. Sadly Clang currently seems to crash if you try to use __has_include in the body of a preprocessor macro.