Changeset 196734 in webkit


Ignore:
Timestamp:
Feb 17, 2016 5:17:36 PM (8 years ago)
Author:
keith_miller@apple.com
Message:

Spread operator should be allowed when not the first argument of parameter list
​https://bugs.webkit.org/show_bug.cgi?id=152721

Reviewed by Saam Barati.

Source/JavaScriptCore:

Spread arguments to functions should now be ES6 compliant. Before we
would only take a spread operator if it was the sole argument to a
function. Additionally, we would not use the Symbol.iterator on the
object to generate the arguments. Instead we would do a loop up to the
length mapping indexed properties to the corresponding argument. We fix
both these issues by doing an AST transformation from foo(...a, b, ...c, d)
to foo(...[...a, b, ...c, d]) (where the spread on the rhs uses the
old spread semantics). This solution has the downside of requiring the
allocation of another object and copying each element twice but avoids a
large change to the vm calling convention.

  • interpreter/Interpreter.cpp:

(JSC::loadVarargs):

  • parser/ASTBuilder.h:

(JSC::ASTBuilder::createElementList):

  • parser/Parser.cpp:

(JSC::Parser<LexerType>::parseArguments):
(JSC::Parser<LexerType>::parseArgument):
(JSC::Parser<LexerType>::parseMemberExpression):

  • parser/Parser.h:
  • parser/SyntaxChecker.h:

(JSC::SyntaxChecker::createElementList):

  • tests/es6.yaml:
  • tests/stress/spread-calling.js: Added.

(testFunction):
(testEmpty):
(makeObject):
(otherIterator.return.next):
(otherIterator):
(totalIter):
(throwingIter.return.next):
(throwingIter):
(i.catch):

LayoutTests:

Update tests with new semantics of spread calling. Additionally,
adjust benchmarks to run in a more reasonable time now that
spread is implemented correctly.

  • js/basic-spread-expected.txt:
  • js/parser-syntax-check-expected.txt:
  • js/regress/script-tests/deltablue-varargs.js:

(deltaBlue):

  • js/regress/script-tests/varargs-construct.js:
  • js/script-tests/basic-spread.js:
  • js/script-tests/parser-syntax-check.js:
Location:
trunk
Files:
1 added
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r196727 r196734  
     12016-02-17  Keith Miller  <keith_miller@apple.com>
     2
     3        Spread operator should be allowed when not the first argument of parameter list
     4        https://bugs.webkit.org/show_bug.cgi?id=152721
     5
     6        Reviewed by Saam Barati.
     7
     8        Update tests with new semantics of spread calling. Additionally,
     9        adjust benchmarks to run in a more reasonable time now that
     10        spread is implemented correctly.
     11
     12        * js/basic-spread-expected.txt:
     13        * js/parser-syntax-check-expected.txt:
     14        * js/regress/script-tests/deltablue-varargs.js:
     15        (deltaBlue):
     16        * js/regress/script-tests/varargs-construct.js:
     17        * js/script-tests/basic-spread.js:
     18        * js/script-tests/parser-syntax-check.js:
     19
    1202016-02-17  Ryan Haddad  <ryanhaddad@apple.com>
    221

  • trunk/LayoutTests/js/basic-spread-expected.txt

    r196704 r196734  
    44
    55
    6 PASS passedThis is o
    7 PASS args[0] is 1
    8 PASS args[1] is undefined
    9 PASS args[2] is null
    10 PASS args[3] is 4
    11 PASS passedThis is o
    12 PASS args[0] is 1
    13 PASS args[1] is undefined
    14 PASS args[2] is null
    15 PASS args[3] is 4
    16 PASS passedThis is o
    17 PASS args[0] is 1
    18 PASS args[1] is undefined
    19 PASS args[2] is null
    20 PASS args[3] is 4
    21 PASS passedThis is o
    22 PASS args[0] is 1
    23 PASS args[1] is undefined
    24 PASS args[2] is null
    25 PASS args[3] is 4
    26 PASS passedThis is o
    27 PASS args[0] is 1
    28 PASS args[1] is undefined
    29 PASS args[2] is null
    30 PASS args[3] is 4
    31 PASS passedThis is o
    32 PASS args[0] is 1
    33 PASS args[1] is undefined
    34 PASS args[2] is null
    35 PASS args[3] is 4
    36 PASS passedThis is o
    37 PASS args[0] is 1
    38 PASS args[1] is undefined
    39 PASS args[2] is null
    40 PASS args[3] is 4
    41 PASS passedThis is o
    42 PASS args[0] is 1
    43 PASS args[1] is undefined
    44 PASS args[2] is null
    45 PASS args[3] is 4
    466PASS passedThis is o
    477PASS args[0] is 1

  • trunk/LayoutTests/js/parser-syntax-check-expected.txt

    r196704 r196734  
    758758PASS Invalid: "o[foo](bar...)"
    759759PASS Invalid: "function f() { o[foo](bar...) }"
    760 PASS Invalid: "foo(a,...bar)"
    761 PASS Invalid: "function f() { foo(a,...bar) }"
    762 PASS Invalid: "o.foo(a,...bar)"
    763 PASS Invalid: "function f() { o.foo(a,...bar) }"
    764 PASS Invalid: "o[foo](a,...bar)"
    765 PASS Invalid: "function f() { o[foo](a,...bar) }"
    766 PASS Invalid: "foo(...bar, a)"
    767 PASS Invalid: "function f() { foo(...bar, a) }"
    768 PASS Invalid: "o.foo(...bar, a)"
    769 PASS Invalid: "function f() { o.foo(...bar, a) }"
    770 PASS Invalid: "o[foo](...bar, a)"
    771 PASS Invalid: "function f() { o[foo](...bar, a) }"
     760PASS Valid:   "foo(a,...bar)" with ReferenceError
     761PASS Valid:  "function f() { foo(a,...bar) }"
     762PASS Valid:   "o.foo(a,...bar)" with ReferenceError
     763PASS Valid:  "function f() { o.foo(a,...bar) }"
     764PASS Valid:   "o[foo](a,...bar)" with ReferenceError
     765PASS Valid:  "function f() { o[foo](a,...bar) }"
     766PASS Valid:   "foo(...bar, a)" with ReferenceError
     767PASS Valid:  "function f() { foo(...bar, a) }"
     768PASS Valid:   "o.foo(...bar, a)" with ReferenceError
     769PASS Valid:  "function f() { o.foo(...bar, a) }"
     770PASS Valid:   "o[foo](...bar, a)" with ReferenceError
     771PASS Valid:  "function f() { o[foo](...bar, a) }"
    772772PASS Valid:   "[...bar]" with ReferenceError
    773773PASS Valid:   "function f() { [...bar] }"

  • trunk/LayoutTests/js/regress/script-tests/deltablue-varargs.js

    r181195 r196734  
    881881
    882882function deltaBlue() {
    883   chainTest(...args(100));
    884   projectionTest(...args(100));
    885 }
    886 
    887 for (var i = 0; i < 30; ++i)
     883  chainTest(...args(25));
     884  projectionTest(...args(25));
     885}
     886
     887for (var i = 0; i < 5; ++i)
    888888    deltaBlue(...args());

  • trunk/LayoutTests/js/regress/script-tests/varargs-construct.js

    r181993 r196734  
    1515noInline(bar);
    1616
    17 for (var i = 0; i < 1000000; ++i) {
     17for (var i = 0; i < 100000; ++i) {
    1818    var result = bar(1, 2);
    1919    if (result.f != 1)

  • trunk/LayoutTests/js/script-tests/basic-spread.js

    r196704 r196734  
    1818var test1 = [1, undefined, null, 4]
    1919var test2 = [1, , null, 4]
    20 var test3 = {length: 4, 0: 1, 2: null, 3: 4}
    21 var test4 = {length: 4, 0: 1, 1: undefined, 2: null, 3: 4}
    2220o.f(...test1)
    2321o.f(...test2)
    24 o.f(...test3)
    25 o.f(...test4)
    2622
    2723var h=eval('"f"')
    2824o[h](...test1)
    2925o[h](...test2)
    30 o[h](...test3)
    31 o[h](...test4)
    3226
    3327function g()
    … …  
    3832g.apply(null, test1)
    3933g.apply(null, test2)
    40 g.apply(null, test3)
    41 g.apply(null, test4)
    4234
    4335g(...test1)
    4436g(...test2)
    45 g(...test3)
    46 g(...test4)
    4737
    4838var a=[1,2,3]

  • trunk/LayoutTests/js/script-tests/parser-syntax-check.js

    r196704 r196734  
    472472invalid("o.foo(bar...)")
    473473invalid("o[foo](bar...)")
    474 invalid("foo(a,...bar)")
    475 invalid("o.foo(a,...bar)")
    476 invalid("o[foo](a,...bar)")
    477 invalid("foo(...bar, a)")
    478 invalid("o.foo(...bar, a)")
    479 invalid("o[foo](...bar, a)")
     474valid("foo(a,...bar)")
     475valid("o.foo(a,...bar)")
     476valid("o[foo](a,...bar)")
     477valid("foo(...bar, a)")
     478valid("o.foo(...bar, a)")
     479valid("o[foo](...bar, a)")
    480480valid("[...bar]")
    481481valid("[a, ...bar]")

  • trunk/Source/JavaScriptCore/ChangeLog

    r196733 r196734  
     12016-02-17  Keith Miller  <keith_miller@apple.com>
     2
     3        Spread operator should be allowed when not the first argument of parameter list
     4        https://bugs.webkit.org/show_bug.cgi?id=152721
     5
     6        Reviewed by Saam Barati.
     7
     8        Spread arguments to functions should now be ES6 compliant. Before we
     9        would only take a spread operator if it was the sole argument to a
     10        function. Additionally, we would not use the Symbol.iterator on the
     11        object to generate the arguments. Instead we would do a loop up to the
     12        length mapping indexed properties to the corresponding argument. We fix
     13        both these issues by doing an AST transformation from foo(...a, b, ...c, d)
     14        to foo(...[...a, b, ...c, d]) (where the spread on the rhs uses the
     15        old spread semantics). This solution has the downside of requiring the
     16        allocation of another object and copying each element twice but avoids a
     17        large change to the vm calling convention.
     18
     19        * interpreter/Interpreter.cpp:
     20        (JSC::loadVarargs):
     21        * parser/ASTBuilder.h:
     22        (JSC::ASTBuilder::createElementList):
     23        * parser/Parser.cpp:
     24        (JSC::Parser<LexerType>::parseArguments):
     25        (JSC::Parser<LexerType>::parseArgument):
     26        (JSC::Parser<LexerType>::parseMemberExpression):
     27        * parser/Parser.h:
     28        * parser/SyntaxChecker.h:
     29        (JSC::SyntaxChecker::createElementList):
     30        * tests/es6.yaml:
     31        * tests/stress/spread-calling.js: Added.
     32        (testFunction):
     33        (testEmpty):
     34        (makeObject):
     35        (otherIterator.return.next):
     36        (otherIterator):
     37        (totalIter):
     38        (throwingIter.return.next):
     39        (throwingIter):
     40        (i.catch):
     41
    1422016-02-17  Brian Burg  <bburg@apple.com>
    243

  • trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

    r196722 r196734  
    248248void loadVarargs(CallFrame* callFrame, VirtualRegister firstElementDest, JSValue arguments, uint32_t offset, uint32_t length)
    249249{
    250     if (UNLIKELY(!arguments.isCell()))
     250    if (UNLIKELY(!arguments.isCell()) || !length)
    251251        return;
    252252   

  • trunk/Source/JavaScriptCore/parser/ASTBuilder.h

    r196704 r196734  
    442442    ElementNode* createElementList(int elisions, ExpressionNode* expr) { return new (m_parserArena) ElementNode(elisions, expr); }
    443443    ElementNode* createElementList(ElementNode* elems, int elisions, ExpressionNode* expr) { return new (m_parserArena) ElementNode(elems, elisions, expr); }
     444    ElementNode* createElementList(ArgumentListNode* elems)
     445    {
     446        ElementNode* head = new (m_parserArena) ElementNode(0, elems->m_expr);
     447        ElementNode* tail = head;
     448        elems = elems->m_next;
     449        while (elems) {
     450            tail = new (m_parserArena) ElementNode(tail, 0, elems->m_expr);
     451            elems = elems->m_next;
     452        }
     453        return head;
     454    }
    444455
    445456    FormalParameterList createFormalParameterList() { return new (m_parserArena) FunctionParameters(); }

  • trunk/Source/JavaScriptCore/parser/Parser.cpp

    r196704 r196734  
    37043704
    37053705template <typename LexerType>
    3706 template <class TreeBuilder> TreeArguments Parser<LexerType>::parseArguments(TreeBuilder& context, SpreadMode mode)
     3706template <class TreeBuilder> TreeArguments Parser<LexerType>::parseArguments(TreeBuilder& context)
    37073707{
    37083708    consumeOrFailWithFlags(OPENPAREN, TreeBuilder::DontBuildStrings, "Expected opening '(' at start of argument list");
    … …  
    37123712        return context.createArguments();
    37133713    }
    3714     if (match(DOTDOTDOT) && mode == AllowSpread) {
     3714    auto argumentsStart = m_token.m_startPosition;
     3715    auto argumentsDivot = m_token.m_endPosition;
     3716
     3717    ArgumentType argType = ArgumentType::Normal;
     3718    TreeExpression firstArg = parseArgument(context, argType);
     3719    failIfFalse(firstArg, "Cannot parse function argument");
     3720    semanticFailIfTrue(match(DOTDOTDOT), "The '...' operator should come before the target expression");
     3721
     3722    bool hasSpread = false;
     3723    if (argType == ArgumentType::Spread)
     3724        hasSpread = true;
     3725    TreeArgumentsList argList = context.createArgumentsList(location, firstArg);
     3726    TreeArgumentsList tail = argList;
     3727
     3728    while (match(COMMA)) {
     3729        JSTokenLocation argumentLocation(tokenLocation());
     3730        next(TreeBuilder::DontBuildStrings);
     3731
     3732        TreeExpression arg = parseArgument(context, argType);
     3733        propagateError();
     3734        semanticFailIfTrue(match(DOTDOTDOT), "The '...' operator should come before the target expression");
     3735
     3736        if (argType == ArgumentType::Spread)
     3737            hasSpread = true;
     3738
     3739        tail = context.createArgumentsList(argumentLocation, tail, arg);
     3740    }
     3741
     3742    handleProductionOrFail(CLOSEPAREN, ")", "end", "argument list");
     3743    if (hasSpread) {
     3744        TreeExpression spreadArray = context.createSpreadExpression(location, context.createArray(location, context.createElementList(argList)), argumentsStart, argumentsDivot, m_lastTokenEndPosition);
     3745        return context.createArguments(context.createArgumentsList(location, spreadArray));
     3746    }
     3747
     3748    return context.createArguments(argList);
     3749}
     3750
     3751template <typename LexerType>
     3752template <class TreeBuilder> TreeExpression Parser<LexerType>::parseArgument(TreeBuilder& context, ArgumentType& type)
     3753{
     3754    if (UNLIKELY(match(DOTDOTDOT))) {
    37153755        JSTokenLocation spreadLocation(tokenLocation());
    37163756        auto start = m_token.m_startPosition;
    37173757        auto divot = m_token.m_endPosition;
    37183758        next();
    3719         auto spreadExpr = parseAssignmentExpression(context);
     3759        TreeExpression spreadExpr = parseAssignmentExpression(context);
     3760        propagateError();
    37203761        auto end = m_lastTokenEndPosition;
    3721         if (!spreadExpr)
    3722             failWithMessage("Cannot parse spread expression");
    3723         if (!consume(CLOSEPAREN)) {
    3724             if (match(COMMA))
    3725                 semanticFail("Spread operator may only be applied to the last argument passed to a function");
    3726             handleProductionOrFail(CLOSEPAREN, ")", "end", "argument list");
    3727         }
    3728         auto spread = context.createSpreadExpression(spreadLocation, spreadExpr, start, divot, end);
    3729         TreeArgumentsList argList = context.createArgumentsList(location, spread);
    3730         return context.createArguments(argList);
    3731     }
    3732     TreeExpression firstArg = parseAssignmentExpression(context);
    3733     failIfFalse(firstArg, "Cannot parse function argument");
    3734    
    3735     TreeArgumentsList argList = context.createArgumentsList(location, firstArg);
    3736     TreeArgumentsList tail = argList;
    3737     while (match(COMMA)) {
    3738         JSTokenLocation argumentLocation(tokenLocation());
    3739         next(TreeBuilder::DontBuildStrings);
    3740         TreeExpression arg = parseAssignmentExpression(context);
    3741         failIfFalse(arg, "Cannot parse function argument");
    3742         tail = context.createArgumentsList(argumentLocation, tail, arg);
    3743     }
    3744     semanticFailIfTrue(match(DOTDOTDOT), "The '...' operator should come before the target expression");
    3745     handleProductionOrFail(CLOSEPAREN, ")", "end", "argument list");
    3746     return context.createArguments(argList);
     3762        type = ArgumentType::Spread;
     3763        return context.createSpreadExpression(spreadLocation, spreadExpr, start, divot, end);
     3764    }
     3765
     3766    type = ArgumentType::Normal;
     3767    return parseAssignmentExpression(context);
    37473768}
    37483769
    … …  
    38153836                newCount--;
    38163837                JSTextPosition expressionEnd = lastTokenEndPosition();
    3817                 TreeArguments arguments = parseArguments(context, AllowSpread);
     3838                TreeArguments arguments = parseArguments(context);
    38183839                failIfFalse(arguments, "Cannot parse call arguments");
    38193840                base = context.createNewExpr(location, base, arguments, expressionStart, expressionEnd, lastTokenEndPosition());
    38203841            } else {
    38213842                JSTextPosition expressionEnd = lastTokenEndPosition();
    3822                 TreeArguments arguments = parseArguments(context, AllowSpread);
     3843                TreeArguments arguments = parseArguments(context);
    38233844                failIfFalse(arguments, "Cannot parse call arguments");
    38243845                if (baseIsSuper)

  • trunk/Source/JavaScriptCore/parser/Parser.h

    r196704 r196734  
    676676    ScopeStack* m_scopeStack;
    677677    unsigned m_index;
     678};
     679
     680enum class ArgumentType {
     681    Normal,
     682    Spread
    678683};
    679684
    … …  
    12401245    template <class TreeBuilder> NEVER_INLINE TreeExpression parseStrictObjectLiteral(TreeBuilder&);
    12411246    template <class TreeBuilder> ALWAYS_INLINE TreeExpression parseFunctionExpression(TreeBuilder&);
    1242     enum SpreadMode { AllowSpread, DontAllowSpread };
    1243     template <class TreeBuilder> ALWAYS_INLINE TreeArguments parseArguments(TreeBuilder&, SpreadMode);
     1247    template <class TreeBuilder> ALWAYS_INLINE TreeArguments parseArguments(TreeBuilder&);
     1248    template <class TreeBuilder> ALWAYS_INLINE TreeExpression parseArgument(TreeBuilder&, ArgumentType&);
    12441249    template <class TreeBuilder> TreeProperty parseProperty(TreeBuilder&, bool strict);
    12451250    template <class TreeBuilder> TreeExpression parsePropertyMethod(TreeBuilder& context, const Identifier* methodName, bool isGenerator);

  • trunk/Source/JavaScriptCore/parser/SyntaxChecker.h

    r196704 r196734  
    222222    int createElementList(int, int) { return ElementsListResult; }
    223223    int createElementList(int, int, int) { return ElementsListResult; }
     224    int createElementList(int) { return ElementsListResult; }
    224225    int createFormalParameterList() { return FormalParameterListResult; }
    225226    void appendParameter(int, DestructuringPattern, int) { }

  • trunk/Source/JavaScriptCore/tests/es6.yaml

    r196722 r196734  
    11141114  cmd: runES6 :normal
    11151115- path: es6/spread_..._operator_with_astral_plane_strings_in_function_calls.js
    1116   cmd: runES6 :fail
     1116  cmd: runES6 :normal
    11171117- path: es6/spread_..._operator_with_generator_instances_in_arrays.js
    11181118  cmd: runES6 :normal
    11191119- path: es6/spread_..._operator_with_generator_instances_in_calls.js
    1120   cmd: runES6 :fail
     1120  cmd: runES6 :normal
    11211121- path: es6/spread_..._operator_with_generic_iterables_in_arrays.js
    11221122  cmd: runES6 :fail
    … …  
    11281128  cmd: runES6 :fail
    11291129- path: es6/spread_..._operator_with_strings_in_function_calls.js
    1130   cmd: runES6 :fail
     1130  cmd: runES6 :normal
    11311131- path: es6/typed_arrays_%TypedArray%.from.js
    11321132  cmd: runES6 :normal
Note: See TracChangeset for help on using the changeset viewer.