Bug 142200

Summary:

Support extends and super keywords

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

JavaScriptCore

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, benjamin, commit-queue, fpizlo, ggaren, glenn, oliver

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

140491, 157972

Attachments:

Description	Flags
WIP	none
Implements the feature	none
Fixed build	none
Updated for ToT and removed redundant code	none
Updated for ToT	none
Addresses Oliver's comment	none
Fixed the merge	none
Fixed per pizlo's comment	fpizlo: review+

Ryosuke Niwa

Reported 2015-03-02 17:41:27 PST

Support extends and super keywords for ES6 class syntax.

Attachments
WIP (51.91 KB, patch) 2015-03-02 17:51 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Implements the feature (80.79 KB, patch) 2015-03-05 04:28 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Fixed build (80.91 KB, patch) 2015-03-05 04:31 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Updated for ToT and removed redundant code (77.92 KB, patch) 2015-03-05 18:07 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Updated for ToT (76.88 KB, patch) 2015-03-06 16:40 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Addresses Oliver's comment (140.64 KB, patch) 2015-03-06 19:42 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Fixed the merge (77.10 KB, patch) 2015-03-06 19:45 PST, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Fixed per pizlo's comment (75.48 KB, patch) 2015-03-09 15:50 PDT, Ryosuke Niwa	fpizlo: review+	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2015-03-02 17:51:40 PST

Created attachment 247727 [details] WIP

Ryosuke Niwa

Comment 2 2015-03-05 04:28:06 PST

Created attachment 247944 [details] Implements the feature

WebKit Commit Bot

Comment 3 2015-03-05 04:30:17 PST

Attachment 247944 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/parser/Nodes.h:1604: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Nodes.h:1606: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:86: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:184: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:557: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Parser.h:373: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] Total errors found: 6 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.

Ryosuke Niwa

Comment 4 2015-03-05 04:30:17 PST

I'll implment TDZ in a separate patch.

Ryosuke Niwa

Comment 5 2015-03-05 04:31:48 PST

Created attachment 247945 [details] Fixed build

WebKit Commit Bot

Comment 6 2015-03-05 04:33:24 PST

Attachment 247945 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/parser/Nodes.h:1604: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Nodes.h:1606: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:86: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:184: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:557: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Parser.h:373: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] Total errors found: 6 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.

Ryosuke Niwa

Comment 7 2015-03-05 18:07:06 PST

Created attachment 248026 [details] Updated for ToT and removed redundant code

WebKit Commit Bot

Comment 8 2015-03-05 18:08:53 PST

Attachment 248026 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/parser/Nodes.h:1604: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Nodes.h:1606: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:86: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:184: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:557: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Parser.h:373: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] Total errors found: 6 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.

Oliver Hunt

Comment 9 2015-03-05 18:11:22 PST

Comment on attachment 248026 [details] Updated for ToT and removed redundant code View in context: https://bugs.webkit.org/attachment.cgi?id=248026&action=review minor minor suggestions from review while building something else > Source/JavaScriptCore/parser/Parser.h:756 > + template <class TreeBuilder> TreeProperty parseGetterSetter(TreeBuilder&, bool strict, PropertyNode::Type, unsigned getterOrSetterStartOffset, bool isMethod = false); Could you make isMethod an enum type? for my own sanity? I regret every bool in this :( > Source/JavaScriptCore/parser/SyntaxChecker.h:239 > + Property createGetterOrSetterProperty(const JSTokenLocation&, PropertyNode::Type type, bool strict, const Identifier* name, const ParserFunctionInfo<SyntaxChecker>&, unsigned, bool) again ends make the world nicer :D

Ryosuke Niwa

Comment 10 2015-03-06 16:40:47 PST

Created attachment 248113 [details] Updated for ToT

Ryosuke Niwa

Comment 11 2015-03-06 16:41:15 PST

(In reply to comment #9) > Comment on attachment 248026 [details] > Updated for ToT and removed redundant code > > View in context: > https://bugs.webkit.org/attachment.cgi?id=248026&action=review > > minor minor suggestions from review while building something else > > > Source/JavaScriptCore/parser/Parser.h:756 > > + template <class TreeBuilder> TreeProperty parseGetterSetter(TreeBuilder&, bool strict, PropertyNode::Type, unsigned getterOrSetterStartOffset, bool isMethod = false); > > Could you make isMethod an enum type? for my own sanity? I regret every bool > in this :( I think I can get rid of this argument altogether.

WebKit Commit Bot

Comment 12 2015-03-06 16:44:09 PST

Attachment 248113 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/parser/Nodes.h:1607: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Nodes.h:1609: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:86: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:184: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:557: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Parser.h:371: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] Total errors found: 6 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.

Ryosuke Niwa

Comment 13 2015-03-06 19:14:04 PST

(In reply to comment #11) > (In reply to comment #9) > > Comment on attachment 248026 [details] > > Updated for ToT and removed redundant code > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=248026&action=review > > > > minor minor suggestions from review while building something else > > > > > Source/JavaScriptCore/parser/Parser.h:756 > > > + template <class TreeBuilder> TreeProperty parseGetterSetter(TreeBuilder&, bool strict, PropertyNode::Type, unsigned getterOrSetterStartOffset, bool isMethod = false); > > > > Could you make isMethod an enum type? for my own sanity? I regret every bool > > in this :( > > I think I can get rid of this argument altogether. Scratch that idea. I'm adding SuperBinding enum instead.

Ryosuke Niwa

Comment 14 2015-03-06 19:42:56 PST

Created attachment 248132 [details] Addresses Oliver's comment

WebKit Commit Bot

Comment 15 2015-03-06 19:45:11 PST

Attachment 248132 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/parser/Nodes.h:1607: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Nodes.h:1609: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:86: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:184: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:557: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Parser.h:371: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] Total errors found: 6 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.

Ryosuke Niwa

Comment 16 2015-03-06 19:45:40 PST

Created attachment 248133 [details] Fixed the merge

WebKit Commit Bot

Comment 17 2015-03-06 19:48:26 PST

Attachment 248133 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/parser/Nodes.h:1607: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Nodes.h:1609: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:86: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:184: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:557: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Parser.h:371: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] Total errors found: 6 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 18 2015-03-09 13:06:24 PDT

Comment on attachment 248133 [details] Fixed the merge View in context: https://bugs.webkit.org/attachment.cgi?id=248133&action=review > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp:88 > +#if ENABLE(ES6_CLASS_SYNTAX) When would we disable this? > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:76 > +#if ENABLE(ES6_CLASS_SYNTAX) > + , m_constructorKindIsDerived(constructorKindIsDerived) > +#endif You could set this unconditionally without the #if. > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:78 > + UNUSED_PARAM(constructorKindIsDerived); ... and RELEASE_ASSERT that it's false if !ENABLE(ES6_CLASS_SYNTAX) > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:87 > +#if ENABLE(ES6_CLASS_SYNTAX) > + bool m_constructorKindIsDerived : 1; > +#endif No need for this field to be conditionalized. > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:130 > +#if ENABLE(ES6_CLASS_SYNTAX) > + bool constructorKindIsDerived() const { return m_constructorKindIsDerived; } > +#else > + bool constructorKindIsDerived() const { return false; } > +#endif If you make the field always present unconditionally, then you don't need this #if here. > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:185 > +#if ENABLE(ES6_CLASS_SYNTAX) > + bool m_constructorKindIsDerived : 1; > +#endif This doesn't need to be conditionalized. > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:363 > +#if ENABLE(ES6_CLASS_SYNTAX) > + bool constructorKindIsDerived() const { return m_constructorKindIsDerived; } > +#endif No need for this to be conditionalized. > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:558 > +#if ENABLE(ES6_CLASS_SYNTAX) > + bool m_constructorKindIsDerived : 1; > +#endif No need for this to be conditionalized. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:405 > +#if ENABLE(ES6_CLASS_SYNTAX) > + if (constructorKindIsDerived()) { No need for this to be conditionalized with an #if; constructorKindIsDerived() will always be false if !ENABLE(ES6_CLASS_SYNTAX). > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1915 > +#if ENABLE(ES6_CLASS_SYNTAX) > + bool thisMightBeUninitialized = constructorKindIsDerived(); > +#else > + bool thisMightBeUninitialized = false; > +#endif No need for conditionalization here. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1921 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:273 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:296 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:772 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:368 > +#if ENABLE(ES6_CLASS_SYNTAX) No need for #if here, you should just ensure that needsSuperBinding() always returns false if the feature is disabled. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:386 > +#if ENABLE(ES6_CLASS_SYNTAX) No need for this condition. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:395 > +#if ENABLE(ES6_CLASS_SYNTAX) No need for this. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:408 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:418 > +#if ENABLE(ES6_CLASS_SYNTAX) > + if (pair.second && pair.second->needsSuperBinding()) Ditto. needsSuperBinding() will already return false if !ES6_CLASS_SYNTAX > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:432 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:503 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:597 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:659 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:670 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:693 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:703 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:1049 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:1063 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Nodes.h:1609 > +#if ENABLE(ES6_CLASS_SYNTAX) > + bool m_constructorKindIsDerived : 1; Ditto - this field should just always be false if !ES6_CLASS_SYNTAX > Source/JavaScriptCore/parser/Parser.cpp:1375 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Parser.cpp:1383 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Parser.cpp:2384 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Parser.cpp:2425 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Parser.cpp:2446 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Parser.cpp:2451 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Parser.h:116 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Parser.h:134 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Parser.h:272 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/Parser.h:370 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto. > Source/JavaScriptCore/parser/SyntaxChecker.h:76 > +#if ENABLE(ES6_CLASS_SYNTAX) It would be great if you could remove this, also - by just allowing ClassExpr and SuperXYZ stuff to just always be declared even if unused. > Source/JavaScriptCore/parser/SyntaxChecker.h:152 > +#if ENABLE(ES6_CLASS_SYNTAX) Ditto.

Ryosuke Niwa

Comment 19 2015-03-09 15:50:21 PDT

Created attachment 248284 [details] Fixed per pizlo's comment

WebKit Commit Bot

Comment 20 2015-03-09 15:53:11 PDT

Attachment 248284 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/parser/Nodes.h:1603: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Nodes.h:1604: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:82: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:173: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:542: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] ERROR: Source/JavaScriptCore/parser/Parser.h:368: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] Total errors found: 6 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 21 2015-03-09 16:02:21 PDT

Comment on attachment 248284 [details] Fixed per pizlo's comment r=me

Ryosuke Niwa

Comment 22 2015-03-09 16:47:31 PDT

Committed r181293: <http://trac.webkit.org/changeset/181293>

Note You need to log in before you can comment on or make changes to this bug.