Bug 74510 - Add ENABLE(HARMONY) and --harmony
Summary: Add ENABLE(HARMONY) and --harmony
Status: RESOLVED INVALID
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: 72960
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-14 09:14 PST by Andy Wingo
Modified: 2011-12-15 07:02 PST (History)
1 user (show)

See Also:


Attachments
Patch (2.89 KB, patch)
2011-12-14 09:18 PST, Andy Wingo
barraclough: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 :)