The following program should be valid, but throws a SyntaxError: ``` class A { super; static; set; get; test() { return "foo"; } } ```
Created attachment 394826 [details] Patch
Comment on attachment 394826 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394826&action=review > Source/JavaScriptCore/ChangeLog:8 > + It allows class fields being created using "static" as identifier. We should link to the spec here but unfortunately the PR is still unmerged. (https://github.com/tc39/ecma262/pull/1668) Since the idea is the same as other identifiers though, I suppose we could at least link to this: https://tc39.es/ecma262/#prod-IdentifierName > Source/JavaScriptCore/parser/Parser.cpp:2880 > + // Reparse "static()" as a method named or "static" as a class field. grammar nit: remove 'named' > JSTests/stress/class-fields-harmony.js:909 > +{ I think Cait brought this file over from V8. Are we sure we want to make arbitrary edits here? (https://github.com/v8/v8/blob/master/test/mjsunit/harmony/public-instance-class-fields.js) Also, perhaps we should add this to test262? Because SM is currently failing too.
Created attachment 394916 [details] Patch
Comment on attachment 394826 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394826&action=review Thank you very much for the review! >> Source/JavaScriptCore/ChangeLog:8 >> + It allows class fields being created using "static" as identifier. > > We should link to the spec here but unfortunately the PR is still unmerged. > (https://github.com/tc39/ecma262/pull/1668) > > Since the idea is the same as other identifiers though, I suppose we could at least link to this: > https://tc39.es/ecma262/#prod-IdentifierName I agree. Added. >> JSTests/stress/class-fields-harmony.js:909 >> +{ > > I think Cait brought this file over from V8. Are we sure we want to make arbitrary edits here? > (https://github.com/v8/v8/blob/master/test/mjsunit/harmony/public-instance-class-fields.js) > > Also, perhaps we should add this to test262? Because SM is currently failing too. I'll place this in a separate file to avoid any issue. Test262 were added by https://github.com/tc39/test262/pull/2550
Comment on attachment 394916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394916&action=review > JSTests/ChangeLog:8 > + * stress/class-fields-harmony.js: Oops, looks like the ChangeLog needs to be regenerated.
Created attachment 394944 [details] Patch
Comment on attachment 394944 [details] Patch Thank you very much for the review!
Committed r259216: <https://trac.webkit.org/changeset/259216> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394944 [details].
<rdar://problem/61067736>