Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(161)

Issue 385553003: Implement handling of arrow functions in the parser (Closed)

Created:
6 years, 5 months ago by aperez
Modified:
6 years, 5 months ago
CC:
v8-dev
Project:
v8
Visibility:
Public.

Description

Implement handling of arrow functions in the parser Arrow functions are parsed from ParseAssignmentExpression(). Handling the parameter list is done by letting ParseConditionalExpression() parse a comma separated list of identifiers, and it returns a tree of BinaryOperation nodes with VariableProxy leaves, or a single VariableProxy if there is only one parameter. When the arrow token "=>" is found, the VariableProxy nodes are passed to ParseArrowFunctionLiteral(), which will then skip parsing the paramaeter list. This avoids having to rewind when the arrow is found and restart parsing the parameter list. Note that the empty parameter list "()" is handled directly in ParsePrimaryExpression(): after is has consumed the opening parenthesis, if a closing parenthesis follows, then the only valid input is an arrow function. In this case, ParsePrimaryExpression() directly calls ParseArrowFunctionLiteral(), to avoid needing to return a sentinel value to signal the empty parameter list. Because it will consume the body of the arrow function, ParseAssignmentExpression() will not see the arrow "=>" token as next, and return the already-parser expression. The implementation is done in ParserBase, so it was needed to do some additions to ParserBase, ParserTraits and PreParserTraits. Some of the glue code can be removed later on when more more functionality is moved to ParserBase. Additionally, this adds a runtime flag "harmony_arrow_functions" (disabled by default); enabling "harmony" will enable it as well. BUG=v8:2700 LOG=N R=marja@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=22320

Patch Set 1 #

Patch Set 2 : Without the fix for ASAN errors #

Patch Set 3 : Version with the fix for the ASAN issue #

Patch Set 4 : Also call params.Dispose() right after calling ParameterListFromExpression() if ( #

Patch Set 5 : Base patch set, makes ASAN choke #

Unified diffs Side-by-side diffs Delta from patch set Stats (+877 lines, -182 lines) Patch
M src/ast.h View 3 chunks +7 lines, -0 lines 0 comments Download
M src/flag-definitions.h View 2 chunks +2 lines, -0 lines 0 comments Download
M src/messages.js View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M src/parser.h View 1 2 3 4 6 chunks +68 lines, -0 lines 0 comments Download
M src/parser.cc View 1 2 3 4 3 chunks +53 lines, -0 lines 0 comments Download
M src/preparser.h View 1 2 3 4 30 chunks +402 lines, -41 lines 0 comments Download
M src/scanner.h View 1 chunk +2 lines, -0 lines 0 comments Download
M src/scanner.cc View 2 chunks +13 lines, -1 line 0 comments Download
M src/token.h View 1 chunk +133 lines, -132 lines 0 comments Download
M test/cctest/test-parsing.cc View 1 2 3 4 23 chunks +196 lines, -8 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
marja
ASAN fix lgtm. Will land this now. For the original review, see https://codereview.chromium.org/160073006.
6 years, 5 months ago (2014-07-10 12:00:03 UTC) #1
marja
6 years, 5 months ago (2014-07-10 12:27:36 UTC) #2
Message was sent while issue was closed.
Committed patchset #4 manually as r22320 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698