Bug 142200 - Support extends and super keywords
Summary: Support extends and super keywords
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 140491
  Show dependency treegraph
 
Reported: 2015-03-02 17:41 PST by Ryosuke Niwa
Modified: 2015-03-09 16:47 PDT (History)
7 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
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>