Bug 144281

Summary: keywords ("super", "delete", etc) should be valid method names
Product: WebKit Reporter: Erik Arvidsson <arv>
Component: JavaScriptCoreAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren, hi, joepeck, keith_miller, mark.lam, msaboff, rniwa, saam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 140491    
Attachments:
Description Flags
[PATCH] Proposed Fix
none
[PATCH] Proposed Fix none

Erik Arvidsson
Reported 2015-04-27 15:13:13 PDT
http://trac.webkit.org/browser/trunk/LayoutTests/js/script-tests/class-syntax-super.js#L40 shouldThrow('x = class extends Base { constructor() { super(); } super() {} }', '"SyntaxError: \'super\' keyword unexpected here"'); If you reformat this: x = class extends Base { constructor() { super(); } super() { } } It should be clear that all this does is define a method named super. There is on restriction on method names like that. Method names can be any keyword.
Attachments
[PATCH] Proposed Fix (18.74 KB, patch)
2016-01-11 14:35 PST, Joseph Pecoraro 		no flags
DetailsFormatted DiffDiff
[PATCH] Proposed Fix (18.88 KB, patch)
2016-01-11 17:42 PST, Joseph Pecoraro 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2015-04-27 15:28:50 PDT
*** This bug has been marked as a duplicate of bug 144282 ***
Ryosuke Niwa
Comment 2 2015-05-01 18:48:44 PDT
Oh wait, this is a separate bug.
Joseph Pecoraro
Comment 3 2016-01-11 11:40:10 PST
I'll take this. I have a simple fix and we ran into this in the inspector recently.
Joseph Pecoraro
Comment 4 2016-01-11 14:35:25 PST
Created attachment 268714 [details] [PATCH] Proposed Fix This catches another bug (bug 152985) and fixes a few issues with keyword method names.
Ryosuke Niwa
Comment 5 2016-01-11 17:24:29 PST
Comment on attachment 268714 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=268714&action=review > Source/JavaScriptCore/parser/Parser.cpp:2158 > + restoreSavePoint(savePoint); > + isStaticMethod = false; Instead of setting a boolean flag back, can we move the condition above into the if condition, initialize isStaticMethod to false, and then set it true when we don't match open paren? That would make this code easier to read without the comment. > Source/JavaScriptCore/parser/Parser.cpp:2184 > + if (!isGenerator && (match(IDENT) || match(STRING) || match(DOUBLE) || match(INTEGER) || match(OPENBRACKET) || m_token.m_type & KeywordTokenFlag)) { Why don't we use isIdentifierOrKeyword instead?
Joseph Pecoraro
Comment 6 2016-01-11 17:41:10 PST
Comment on attachment 268714 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=268714&action=review >> Source/JavaScriptCore/parser/Parser.cpp:2184 >> + if (!isGenerator && (match(IDENT) || match(STRING) || match(DOUBLE) || match(INTEGER) || match(OPENBRACKET) || m_token.m_type & KeywordTokenFlag)) { > > Why don't we use isIdentifierOrKeyword instead? Awesome! I didn't see this existed. Here we can use matchIdentifierOrKeyword()!
Joseph Pecoraro
Comment 7 2016-01-11 17:42:35 PST
Created attachment 268734 [details] [PATCH] Proposed Fix
Ryosuke Niwa
Comment 8 2016-01-11 18:13:32 PST
Comment on attachment 268734 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=268734&action=review > Source/JavaScriptCore/parser/Parser.cpp:2203 > + if (m_token.m_type & KeywordTokenFlag) > + goto namedKeyword; It's annoying that we have to use goto here but I can't think of a better way.
Joseph Pecoraro
Comment 9 2016-01-11 18:57:59 PST
Comment on attachment 268734 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=268734&action=review >> Source/JavaScriptCore/parser/Parser.cpp:2203 >> + goto namedKeyword; > > It's annoying that we have to use goto here but I can't think of a better way. Yeah. `goto` matched the parseProperty handling of keywords in object literals, so I just matched that and used a goto here. We could convert to a bunch of if/else statements, but I'm not sure that would be any clearer.
WebKit Commit Bot
Comment 10 2016-01-11 19:02:23 PST
Comment on attachment 268734 [details] [PATCH] Proposed Fix Clearing flags on attachment: 268734 Committed r194881: <http://trac.webkit.org/changeset/194881>
WebKit Commit Bot
Comment 11 2016-01-11 19:02:27 PST
All reviewed patches have been landed. Closing bug.
Devin Rousso
Comment 12 2016-01-20 21:32:14 PST
*** Bug 153296 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.