Bug 142200 – Support extends and super keywords

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Ryosuke Niwa]

Support extends and super keywords for ES6 class syntax.

[Ryosuke Niwa]

Created attachment 247727 [details]
WIP

[Ryosuke Niwa]

Created attachment 247944 [details]
Implements the feature

[WebKit Commit Bot]

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]

I'll implment TDZ in a separate patch.

[Ryosuke Niwa]

Created attachment 247945 [details]
Fixed build

[WebKit Commit Bot]

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]

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

[WebKit Commit Bot]

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 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]

Created attachment 248113 [details]
Updated for ToT

[Ryosuke Niwa]

(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]

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]

(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]

Created attachment 248132 [details]
Addresses Oliver's comment

[WebKit Commit Bot]

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]

Created attachment 248133 [details]
Fixed the merge

[WebKit Commit Bot]

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 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]

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

[WebKit Commit Bot]

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 on attachment 248284 [details]
Fixed per pizlo's comment

r=me

[Ryosuke Niwa]

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