Bug 142200

Summary: Support extends and super keywords
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: JavaScriptCoreAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, commit-queue, fpizlo, ggaren, glenn, oliver
Priority: P2    
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+

Description Ryosuke Niwa 2015-03-02 17:41:27 PST 
Support extends and super keywords for ES6 class syntax.
Comment 1 Ryosuke Niwa 2015-03-02 17:51:40 PST 
Created attachment 247727 [details]
WIP
Comment 2 Ryosuke Niwa 2015-03-05 04:28:06 PST 
Created attachment 247944 [details]
Implements the feature
Comment 3 WebKit Commit Bot 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.
Comment 4 Ryosuke Niwa 2015-03-05 04:30:17 PST 
I'll implment TDZ in a separate patch.
Comment 5 Ryosuke Niwa 2015-03-05 04:31:48 PST 
Created attachment 247945 [details]
Fixed build
Comment 6 WebKit Commit Bot 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.
Comment 7 Ryosuke Niwa 2015-03-05 18:07:06 PST 
Created attachment 248026 [details]
Updated for ToT and removed redundant code
Comment 8 WebKit Commit Bot 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.
Comment 9 Oliver Hunt 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
Comment 10 Ryosuke Niwa 2015-03-06 16:40:47 PST 
Created attachment 248113 [details]
Updated for ToT
Comment 11 Ryosuke Niwa 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.
Comment 12 WebKit Commit Bot 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.
Comment 13 Ryosuke Niwa 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.
Comment 14 Ryosuke Niwa 2015-03-06 19:42:56 PST 
Created attachment 248132 [details]
Addresses Oliver's comment
Comment 15 WebKit Commit Bot 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.
Comment 16 Ryosuke Niwa 2015-03-06 19:45:40 PST 
Created attachment 248133 [details]
Fixed the merge
Comment 17 WebKit Commit Bot 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.
Comment 18 Filip Pizlo 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.
Comment 19 Ryosuke Niwa 2015-03-09 15:50:21 PDT 
Created attachment 248284 [details]
Fixed per pizlo's comment
Comment 20 WebKit Commit Bot 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.
Comment 21 Filip Pizlo 2015-03-09 16:02:21 PDT 
Comment on attachment 248284 [details]
Fixed per pizlo's comment

r=me
Comment 22 Ryosuke Niwa 2015-03-09 16:47:31 PDT 
Committed r181293: <http://trac.webkit.org/changeset/181293>