Bug 147090 - Parser::parseFunctionInfo hits RELEASE_ASSERT for Arrow Functions
Summary: Parser::parseFunctionInfo hits RELEASE_ASSERT for Arrow Functions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-19 10:51 PDT by Saam Barati
Modified: 2015-07-19 12:32 PDT (History)
10 users (show)

See Also:


Attachments
patch (4.85 KB, patch)
2015-07-19 11:13 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

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