Bug 70060 - DFG JIT should not be using ENABLE macro to enable features
Summary: DFG JIT should not be using ENABLE macro to enable features
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-13 14:49 PDT by Gavin Barraclough
Modified: 2011-10-13 15:13 PDT (History)
1 user (show)

See Also:


Attachments
The patch (37.73 KB, patch)
2011-10-13 14:51 PDT, Gavin Barraclough
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2011-10-13 14:49:04 PDT
The ENABLE macro is only intended to be used to detect features that are configured in Platform.h.  Using its to detect settings defined in other headers is an error.

The problem is that the ENABLE macro checks if the value is defined, so will silently return false if you fail to include the header defining the switch.
This is not a problem if (1) the settings are defined in the same header that defines the macro that tests them, or (2) the header is included everywhere.  In the case of ENABLE settings defined in Platform.h, both are true!

To make this clear, add an explicit DFG_ENABLE macro.
Comment 1 Gavin Barraclough 2011-10-13 14:51:21 PDT
Created attachment 110913 [details]
The patch
Comment 2 Oliver Hunt 2011-10-13 14:52:50 PDT
Comment on attachment 110913 [details]
The patch

rs=me
Comment 3 WebKit Review Bot 2011-10-13 14:54:18 PDT
Attachment 110913 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/dfg/DFGCapabilities.h:32:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Gavin Barraclough 2011-10-13 15:13:30 PDT
Fixed in r97408