Bug 147090

Summary: Parser::parseFunctionInfo hits RELEASE_ASSERT for Arrow Functions
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: basile_clement, benjamin, commit-queue, fpizlo, ggaren, mark.lam, mmirman, msaboff, oliver, ysuzuki
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch none

Description Saam Barati 2015-07-19 10:51:41 PDT
This assertion hits: "RELEASE_ASSERT(mode == NormalFunctionMode || mode == MethodMode);"
because this is how we parse arrow functions:

template <typename LexerType>
template <class TreeBuilder> TreeExpression Parser<LexerType>::parseArrowFunctionExpression(TreeBuilder& context)
{
    JSTokenLocation location;

    unsigned functionKeywordStart = tokenStart();
    location = tokenLocation();
    ParserFunctionInfo<TreeBuilder> info;
    info.name = &m_vm->propertyNames->nullIdentifier;
    failIfFalse((parseFunctionInfo(context, FunctionNoRequirements, ArrowFunctionMode, true, ConstructorKind::None, SuperBinding::NotNeeded, functionKeywordStart, info, ArrowFunctionParseType)), "Cannot parse arrow function expression");

    return context.createArrowFunctionExpr(location, info);
}

We need to remove this line from the function:
"info.name = &m_vm->propertyNames->nullIdentifier;"
Comment 1 Saam Barati 2015-07-19 11:01:16 PDT
(In reply to comment #0)
> This assertion hits: "RELEASE_ASSERT(mode == NormalFunctionMode || mode ==
> MethodMode);"
> because this is how we parse arrow functions:
> 
> template <typename LexerType>
> template <class TreeBuilder> TreeExpression
> Parser<LexerType>::parseArrowFunctionExpression(TreeBuilder& context)
> {
>     JSTokenLocation location;
> 
>     unsigned functionKeywordStart = tokenStart();
>     location = tokenLocation();
>     ParserFunctionInfo<TreeBuilder> info;
>     info.name = &m_vm->propertyNames->nullIdentifier;
>     failIfFalse((parseFunctionInfo(context, FunctionNoRequirements,
> ArrowFunctionMode, true, ConstructorKind::None, SuperBinding::NotNeeded,
> functionKeywordStart, info, ArrowFunctionParseType)), "Cannot parse arrow
> function expression");
> 
>     return context.createArrowFunctionExpr(location, info);
> }
> 
> We need to remove this line from the function:
> "info.name = &m_vm->propertyNames->nullIdentifier;"

This is a wrong assessment. We need to assign the nullIdentifier and allow 
the assert to not disqualify arrow functions.
Comment 2 Saam Barati 2015-07-19 11:13:27 PDT
Created attachment 257063 [details]
patch
Comment 3 Yusuke Suzuki 2015-07-19 11:42:05 PDT
Comment on attachment 257063 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257063&action=review

r+

> Source/JavaScriptCore/parser/Parser.cpp:1716
>      if (functionScope->strictMode() && functionInfo.name) {

Even arrow function has functionInfo.name (nullIdentifier).
We need to refactor the use cases for functionInfo.name's nullptr-ness.
Comment 4 WebKit Commit Bot 2015-07-19 12:32:53 PDT
Comment on attachment 257063 [details]
patch

Clearing flags on attachment: 257063

Committed r187014: <http://trac.webkit.org/changeset/187014>
Comment 5 WebKit Commit Bot 2015-07-19 12:32:56 PDT
All reviewed patches have been landed.  Closing bug.