Bug 172598 - Prevent async methods named 'function'
Summary: Prevent async methods named 'function'
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: GSkachkov
URL:
Keywords: InRadar
Depends on:
Blocks: 172660 172776
  Show dependency treegraph
 
Reported: 2017-05-25 10:45 PDT by Saam Barati
Modified: 2017-05-31 14:10 PDT (History)
17 users (show)

See Also:


Attachments
Patch (6.31 KB, patch)
2017-05-25 12:59 PDT, GSkachkov
no flags Details | Formatted Diff | Diff
Patch (6.45 KB, patch)
2017-05-25 13:12 PDT, GSkachkov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2017-05-25 10:45:39 PDT
See https://github.com/tc39/ecma262/pull/884?
Comment 1 GSkachkov 2017-05-25 12:59:52 PDT
Created attachment 311271 [details]
Patch

Patch
Comment 2 Mark Lam 2017-05-25 13:06:01 PDT
Comment on attachment 311271 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:3
> +        Prevent async methods named âfunctionâ

Please fix non-ascii characters in the description.

Please also add a url to the spec or maybe  https://github.com/tc39/ecma262/pull/884 that specifies this behavior.
Comment 3 Mark Lam 2017-05-25 13:06:24 PDT
Comment on attachment 311271 [details]
Patch

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

> JSTests/ChangeLog:3
> +        Prevent async methods named âfunctionâ

Ditto for non-ascii characters.
Comment 4 GSkachkov 2017-05-25 13:12:37 PDT
Created attachment 311274 [details]
Patch

Patch with fixes
Comment 5 GSkachkov 2017-05-25 13:13:12 PDT
Comment on attachment 311271 [details]
Patch

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

>> Source/JavaScriptCore/ChangeLog:3
>> +        Prevent async methods named âfunctionâ
> 
> Please fix non-ascii characters in the description.
> 
> Please also add a url to the spec or maybe  https://github.com/tc39/ecma262/pull/884 that specifies this behavior.

Fixed

>> JSTests/ChangeLog:3
>> +        Prevent async methods named âfunctionâ
> 
> Ditto for non-ascii characters.

Oh, it seems that taken from task name. Fixed
Comment 6 Mark Lam 2017-05-25 13:19:25 PDT
Comment on attachment 311274 [details]
Patch

r=me
Comment 7 WebKit Commit Bot 2017-05-26 00:00:48 PDT
Comment on attachment 311274 [details]
Patch

Clearing flags on attachment: 311274

Committed r217478: <http://trac.webkit.org/changeset/217478>
Comment 8 WebKit Commit Bot 2017-05-26 00:00:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Saam Barati 2017-05-26 01:34:28 PDT
Comment on attachment 311274 [details]
Patch

Does this effect Object liberals? If so, please add a test
Comment 10 GSkachkov 2017-05-26 13:03:01 PDT
Yeah?(In reply to Saam Barati from comment #9)
> Comment on attachment 311274 [details]
> Patch
> 
> Does this effect Object liberals? If so, please add a test

Yeah you are right, I was focused on class methods, but spec cover object literals as well https://github.com/tc39/test262/pull/967/files#diff-0fc4f159cc11cd6471944c682729ff0dR17

I've prepared fix, expect to upload it to separate issue tomorrow.
Thanks for comment.
Comment 11 GSkachkov 2017-05-28 13:01:26 PDT
(In reply to Saam Barati from comment #9)
> Comment on attachment 311274 [details]
> Patch
> 
> Does this effect Object liberals? If so, please add a test

There is a patch that cover object literals https://bugs.webkit.org/show_bug.cgi?id=172660
Comment 12 Radar WebKit Bug Importer 2017-05-30 20:21:43 PDT
<rdar://problem/32479725>
Comment 13 Daniel Ehrenberg 2017-05-31 07:56:05 PDT
We reviewed this PR at TC39 and decided *not* to do this spec change, and to leave things as is, permitting async methods named function. Sorry for the confusion.
Comment 14 JF Bastien 2017-05-31 10:15:19 PDT
(In reply to Daniel Ehrenberg from comment #13)
> We reviewed this PR at TC39 and decided *not* to do this spec change, and to
> leave things as is, permitting async methods named function. Sorry for the
> confusion.

Thanks for catching this!

Reference: https://github.com/tc39/ecma262/pull/884
Comment 15 GSkachkov 2017-05-31 11:30:12 PDT
(In reply to Daniel Ehrenberg from comment #13)
> We reviewed this PR at TC39 and decided *not* to do this spec change, and to
> leave things as is, permitting async methods named function. Sorry for the
> confusion.

Ups. Will do rollback. Thanks for update.