Bug 74510

Summary: Add ENABLE(HARMONY) and --harmony
Product: WebKit Reporter: Andy Wingo <wingo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: peter
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 72960    
Bug Blocks:    
Attachments:
Description Flags
Patch barraclough: review-

Description Andy Wingo 2011-12-14 09:14:17 PST
The ENABLE_HARMONY cpp flag, off by default, will control compilation of Harmony features.  The patch to be attached also adds runtime --harmony and --harmony-block-scoping arguments that are available when ENABLE(HARMONY).

See bugĀ 31813 for a broader discussion.  The runtime flags depend on the work from bug 72960, so there are some other patches to review first.
Comment 1 Andy Wingo 2011-12-14 09:18:26 PST
Created attachment 119232 [details]
Patch
Comment 2 Gavin Barraclough 2011-12-14 20:59:21 PST
Comment on attachment 119232 [details]
Patch

I'm not sure that this makes sense as a global setting.  Some scripts may be harmonious, others not, so we're going to need harmony to be enabled on a source-by-source basis, not as a global option.  I don't think a 'harmony' flag in the JSC framework is needed.  The command shell may want to look for this flag as an instruction as to how the source it reads in should be interpreted - to create SourceCode/SourceProvider objects with an ES6 flag set (perhaps this should be a new default-to-false argument to makeSource).

I don't know what you'd do with a harmonyBlockScoping global setting - for ES5 scripts it only makes sense to not use harmony scoping rules, for ES6 scripts it only makes sense to do so.

We generally don't accept changes that are not complete in their own right, and are effectively dead code - which this is.  Whilst we do like to see changes broken down into smaller patches where sensible, I'd suggest that this is a small enough change that it may be wiser to introduce this alongside the first actual harmony feature implemented.
Comment 3 Andy Wingo 2011-12-15 07:02:46 PST
(In reply to comment #2)
> (From update of attachment 119232 [details])
>The command shell may want to look for this flag as an instruction as to how the source it reads in should be interpreted - to create SourceCode/SourceProvider objects with an ES6 flag set (perhaps this should be a new default-to-false argument to makeSource).

This can make sense, yes.

> I don't know what you'd do with a harmonyBlockScoping global setting - for ES5 scripts it only makes sense to not use harmony scoping rules, for ES6 scripts it only makes sense to do so.

Block scoping is actually a compatible change, as far as strict-mode ES5 goes, so it can make sense to say "ES5 strict-mode also has the compatible parts of ES6" -- the "compatible extended mode" I mentioned in https://lists.webkit.org/pipermail/webkit-dev/2011-December/018903.html.

> I'd suggest that this is a small enough change that it may be wiser to introduce this alongside the first actual harmony feature implemented.

Fair enough.

Thanks for the review :)