Bug 108745 - WTF shouldn't use a script build phase to detect the presence of headers when the compiler can do it for us
Summary: WTF shouldn't use a script build phase to detect the presence of headers when...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Rowe (bdash)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-01 23:44 PST by Mark Rowe (bdash)
Modified: 2016-01-31 15:10 PST (History)
5 users (show)

See Also:

Attachments
Patch v1 (7.55 KB, patch)
2013-02-01 23:53 PST, Mark Rowe (bdash) 		eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch v2 (7.64 KB, patch)
2013-02-02 00:51 PST, Mark Rowe (bdash) 		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 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.