Bug 164829 - [JSC] enable ES2017_ASYNCFUNCTION_SYNTAX feature flag
Summary: [JSC] enable ES2017_ASYNCFUNCTION_SYNTAX feature flag
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Caitlin Potter (:caitp)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-16 12:20 PST by Caitlin Potter (:caitp)
Modified: 2018-02-14 10:35 PST (History)
3 users (show)

See Also:


Attachments
Patch (56.89 KB, patch)
2016-11-16 12:25 PST, Caitlin Potter (:caitp)
beidson: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Caitlin Potter (:caitp) 2016-11-16 12:20:17 PST
[JSC] enable ES2017_ASYNCFUNCTION_SYNTAX feature flag
Comment 1 Caitlin Potter (:caitp) 2016-11-16 12:25:45 PST
Created attachment 294960 [details]
Patch
Comment 2 Saam Barati 2016-11-16 12:49:42 PST
Comment on attachment 294960 [details]
Patch

my vote is to remove the compile time flag.
Comment 3 Yusuke Suzuki 2016-11-16 12:50:03 PST
Comment on attachment 294960 [details]
Patch

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

I'm ok. I want Saam's review too :)

> JSTests/ChangeLog:21
> +        * test262.yaml:

Async functions' YAML file should be dropped.
Comment 4 Caitlin Potter (:caitp) 2016-11-16 12:58:52 PST
Comment on attachment 294960 [details]
Patch

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

>> JSTests/ChangeLog:21
>> +        * test262.yaml:
> 
> Async functions' YAML file should be dropped.

Personally, I'd prefer to keep it until I'm no longer working on this feature, it saves a lot of time maintaining it. Up to you guys tho.
Comment 5 Caitlin Potter (:caitp) 2016-11-16 13:01:38 PST
(In reply to comment #2)
> Comment on attachment 294960 [details]
> Patch
> 
> my vote is to remove the compile time flag.

I would suggest landing a flag flip CL first, with a second patch to remove the flag entirely separately, so that it's easier to go back to a flagged status if needed
Comment 6 Yusuke Suzuki 2016-11-16 19:06:43 PST
(In reply to comment #5)
> (In reply to comment #2)
> > Comment on attachment 294960 [details]
> > Patch
> > 
> > my vote is to remove the compile time flag.
> 
> I would suggest landing a flag flip CL first, with a second patch to remove
> the flag entirely separately, so that it's easier to go back to a flagged
> status if needed

Dropping the compile time flag is not difficult to be reverted.
So I like the suggestion dropping the compile time flag in this patch.

If the compile time flag is enabled,
disabled version tends to be broken soon (Unfortunately!).
So even if we have the compile time flag,
disabling it may take some more work beyond flipping the flag.
Comment 7 Brady Eidson 2018-02-14 10:35:49 PST
Comment on attachment 294960 [details]
Patch

Patches that have been up for review since 2016 are almost certainly too stale to be relevant to trunk in their current form.

If this patch is still important please rebase it and post it for review again.