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

Issue 1841543003: [esnext] implement frontend changes for async/await proposal (Closed)

Created:
4 years, 8 months ago by caitp (gmail)
Modified:
4 years, 7 months ago
Reviewers:
Dan Ehrenberg, adamk
CC:
mike3, v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[esnext] implement frontend changes for async/await proposal BUG=v8:4483 LOG=Y R=littledan@chromium.org, adamk@chromium.org Committed: https://crrev.com/0d43421a228b53ecf0e3a176e65618a35b047da6 Cr-Commit-Position: refs/heads/master@{#36261}

Patch Set 1 #

Patch Set 2 : support arrow functions / methods / etc #

Patch Set 3 : Fix up a bit more #

Patch Set 4 : fixup AsyncFunctionExpression parsing #

Patch Set 5 : Make parsing tests all pass #

Patch Set 6 : 2.5 week rebase #

Patch Set 7 : rebase #

Patch Set 8 : rebase over generator change cl #

Total comments: 2

Patch Set 9 : rebase :> #

Patch Set 10 : rebase... maybe will split the `un-boolean-trap`-ization into a separate patch to make rebasing eas… #

Patch Set 11 : "await" is no longer a contextual keyword due to Mikes changes #

Patch Set 12 : finish fixing up after mike's CL #

Patch Set 13 : Rebase for dependent CLs #

Total comments: 17

Patch Set 14 : A bunch more tests, some fixes, ExpressionClassifier gets fatter :( #

Total comments: 11

Patch Set 15 : Rebase + little bit of cleanup #

Patch Set 16 : make a (future) test262 failure pass, but the error message is weird and makes no sense #

Patch Set 17 : super frequent rebasing to keep dependent patchsets clean! #

Patch Set 18 : Fix html comment parsing #

Patch Set 19 : #

Patch Set 20 : make non-clang happy #

Patch Set 21 : Fix more problems #

Total comments: 4

Patch Set 22 : Tokenize "async" as keyword, and other things #

Patch Set 23 : Rebase it too #

Patch Set 24 : Fix a pointless edit #

Total comments: 5

Patch Set 25 : Scanner changes and some other stuff #

Patch Set 26 : Remove more dead code from scanner #

Patch Set 27 : Remove pointless line from scanner #

Total comments: 4

Patch Set 28 : #

Patch Set 29 : #

Patch Set 30 : frontend-side prep #

Total comments: 1

Patch Set 31 : Ban `await` as function IdentifierName inside AsyncFunctions, regardless of function declaration ki… #

Patch Set 32 : Add some module parsing stuff, and that function BindingIdentifier fix #

Total comments: 2

Patch Set 33 : Uncomment that test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+977 lines, -170 lines) Patch
M src/ast/ast.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 4 chunks +14 lines, -16 lines 0 comments Download
M src/ast/ast-value-factory.h View 1 chunk +2 lines, -0 lines 0 comments Download
M src/bootstrapper.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 2 chunks +2 lines, -0 lines 0 comments Download
M src/flag-definitions.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 1 chunk +9 lines, -7 lines 0 comments Download
M src/globals.h View 1 2 3 4 5 6 7 8 9 10 11 19 20 21 3 chunks +11 lines, -1 line 0 comments Download
M src/messages.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +5 lines, -0 lines 0 comments Download
M src/objects.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 4 chunks +10 lines, -2 lines 0 comments Download
M src/objects-inl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 2 chunks +5 lines, -0 lines 0 comments Download
M src/parsing/expression-classifier.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +49 lines, -3 lines 0 comments Download
M src/parsing/parser.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 6 chunks +17 lines, -1 line 0 comments Download
M src/parsing/parser.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 19 chunks +150 lines, -20 lines 0 comments Download
M src/parsing/parser-base.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 59 chunks +338 lines, -102 lines 0 comments Download
M src/parsing/preparser.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 10 chunks +35 lines, -2 lines 0 comments Download
M src/parsing/preparser.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 7 chunks +82 lines, -15 lines 0 comments Download
M src/parsing/scanner.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 28 2 chunks +7 lines, -0 lines 0 comments Download
M src/parsing/scanner.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 28 3 chunks +7 lines, -0 lines 0 comments Download
M src/parsing/token.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +2 lines, -0 lines 0 comments Download
M test/cctest/test-parsing.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 3 chunks +232 lines, -1 line 0 comments Download

Dependent Patchsets:

Messages

Total messages: 49 (7 generated)
caitp (gmail)
Hi, I thought I'd get the ball rolling on the frontend-side of this proposal. As ...
4 years, 8 months ago (2016-03-29 20:06:27 UTC) #1
caitp (gmail)
/CC mike
4 years, 8 months ago (2016-03-31 21:08:54 UTC) #3
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1841543003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1841543003/160001
4 years, 8 months ago (2016-04-22 06:32:11 UTC) #6
Michael Achenbach
Please upload a new (empty) patchset and queue that one (see http://crbug.com/605846). Sorry for the ...
4 years, 8 months ago (2016-04-22 07:32:48 UTC) #7
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on tryserver.v8 (JOB_TIMED_OUT, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng/builds/648) v8_win_rel_ng on ...
4 years, 8 months ago (2016-04-22 08:36:42 UTC) #9
caitp (gmail)
On 2016/04/22 07:32:48, Michael Achenbach wrote: > Please upload a new (empty) patchset and queue ...
4 years, 8 months ago (2016-04-24 23:58:06 UTC) #10
Dan Ehrenberg
https://codereview.chromium.org/1841543003/diff/160001/src/parsing/parser.cc File src/parsing/parser.cc (right): https://codereview.chromium.org/1841543003/diff/160001/src/parsing/parser.cc#newcode1281 src/parsing/parser.cc:1281: PeekContextualKeyword(CStrVector("async")) && You mentioned that this patch causes a ...
4 years, 8 months ago (2016-04-25 21:18:16 UTC) #11
caitp (gmail)
https://codereview.chromium.org/1841543003/diff/160001/src/parsing/parser.cc File src/parsing/parser.cc (right): https://codereview.chromium.org/1841543003/diff/160001/src/parsing/parser.cc#newcode1281 src/parsing/parser.cc:1281: PeekContextualKeyword(CStrVector("async")) && On 2016/04/25 21:18:16, Dan Ehrenberg wrote: > ...
4 years, 8 months ago (2016-04-26 00:09:38 UTC) #12
Dan Ehrenberg
Just a first pass, not a full review https://codereview.chromium.org/1841543003/diff/260001/src/parsing/parser-base.h File src/parsing/parser-base.h (left): https://codereview.chromium.org/1841543003/diff/260001/src/parsing/parser-base.h#oldcode1309 src/parsing/parser-base.h:1309: Spurious ...
4 years, 7 months ago (2016-05-05 01:14:40 UTC) #13
caitp (gmail)
Rest of the comments on the tests are good, I'll work on these tomorrow. Thanks ...
4 years, 7 months ago (2016-05-05 01:36:58 UTC) #14
caitp (gmail)
https://codereview.chromium.org/1841543003/diff/260001/src/parsing/scanner.cc File src/parsing/scanner.cc (right): https://codereview.chromium.org/1841543003/diff/260001/src/parsing/scanner.cc#newcode285 src/parsing/scanner.cc:285: has_multiline_comment_before_next_ = has_multiline_comment_before_next; On 2016/05/05 01:14:40, Dan Ehrenberg wrote: ...
4 years, 7 months ago (2016-05-05 21:49:37 UTC) #15
Dan Ehrenberg
It would be nice to have 'messages' tests for the new syntax errors as well. ...
4 years, 7 months ago (2016-05-06 00:08:56 UTC) #16
caitp (gmail)
thanks, will do a bit more cleanup shortly https://codereview.chromium.org/1841543003/diff/280001/src/parsing/preparser.cc File src/parsing/preparser.cc (right): https://codereview.chromium.org/1841543003/diff/280001/src/parsing/preparser.cc#newcode72 src/parsing/preparser.cc:72: } ...
4 years, 7 months ago (2016-05-06 00:31:03 UTC) #17
caitp (gmail)
I've added some Octane measurements in https://gist.github.com/anonymous/27980d70f09d24c64c27d3ee29e217cd There's about a 2.5% regression in CodeLoad (on ...
4 years, 7 months ago (2016-05-10 14:08:25 UTC) #18
Dan Ehrenberg
Looking forward to the keyword version https://codereview.chromium.org/1841543003/diff/420001/src/globals.h File src/globals.h (right): https://codereview.chromium.org/1841543003/diff/420001/src/globals.h#newcode1079 src/globals.h:1079: }; This seems ...
4 years, 7 months ago (2016-05-11 21:04:17 UTC) #19
caitp (gmail)
https://codereview.chromium.org/1841543003/diff/420001/src/globals.h File src/globals.h (right): https://codereview.chromium.org/1841543003/diff/420001/src/globals.h#newcode1079 src/globals.h:1079: }; On 2016/05/11 21:04:17, Dan Ehrenberg wrote: > This ...
4 years, 7 months ago (2016-05-11 21:14:08 UTC) #20
Dan Ehrenberg
Things are really looking good in this patch; close to being landable IMO. https://codereview.chromium.org/1841543003/diff/480001/src/ast/ast.h File ...
4 years, 7 months ago (2016-05-12 19:24:32 UTC) #21
caitp (gmail)
https://codereview.chromium.org/1841543003/diff/480001/src/parsing/scanner.cc File src/parsing/scanner.cc (right): https://codereview.chromium.org/1841543003/diff/480001/src/parsing/scanner.cc#newcode280 src/parsing/scanner.cc:280: has_preceding_line_terminator_ || has_preceding_multiline_comment_; On 2016/05/12 19:24:32, Dan Ehrenberg wrote: ...
4 years, 7 months ago (2016-05-12 19:56:05 UTC) #22
caitp (gmail)
On 2016/05/12 19:56:05, caitp wrote: > https://codereview.chromium.org/1841543003/diff/480001/src/parsing/scanner.cc > File src/parsing/scanner.cc (right): > > https://codereview.chromium.org/1841543003/diff/480001/src/parsing/scanner.cc#newcode280 > ...
4 years, 7 months ago (2016-05-12 20:39:17 UTC) #23
Dan Ehrenberg
https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h File src/parsing/parser-base.h (right): https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h#newcode380 src/parsing/parser-base.h:380: } How could this strategy detect an error in ...
4 years, 7 months ago (2016-05-12 23:58:29 UTC) #24
caitp (gmail)
https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h File src/parsing/parser-base.h (right): https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h#newcode380 src/parsing/parser-base.h:380: } On 2016/05/12 23:58:29, Dan Ehrenberg wrote: > How ...
4 years, 7 months ago (2016-05-13 00:03:07 UTC) #25
Dan Ehrenberg
https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h File src/parsing/parser-base.h (right): https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h#newcode380 src/parsing/parser-base.h:380: } On 2016/05/13 at 00:03:07, caitp wrote: > On ...
4 years, 7 months ago (2016-05-13 00:14:09 UTC) #26
caitp (gmail)
https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h File src/parsing/parser-base.h (right): https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h#newcode380 src/parsing/parser-base.h:380: } On 2016/05/13 00:14:08, Dan Ehrenberg wrote: > On ...
4 years, 7 months ago (2016-05-13 01:28:40 UTC) #27
Dan Ehrenberg
There are more places where 'await' has to be banned as an identifier: ParserBase<Traits>::ParseIdentifierOrStrictReservedWord (e.g., ...
4 years, 7 months ago (2016-05-13 01:37:13 UTC) #28
caitp (gmail)
On 2016/05/13 01:28:40, caitp wrote: > https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h > File src/parsing/parser-base.h (right): > > https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h#newcode380 > ...
4 years, 7 months ago (2016-05-13 01:44:52 UTC) #29
Dan Ehrenberg
On 2016/05/13 at 01:44:52, caitpotter88 wrote: > On 2016/05/13 01:28:40, caitp wrote: > > https://codereview.chromium.org/1841543003/diff/530001/src/parsing/parser-base.h ...
4 years, 7 months ago (2016-05-13 19:28:11 UTC) #31
caitp (gmail)
On 2016/05/13 19:28:11, Dan Ehrenberg wrote: > On 2016/05/13 at 01:44:52, caitpotter88 wrote: > > ...
4 years, 7 months ago (2016-05-13 19:43:02 UTC) #32
Dan Ehrenberg
On 2016/05/13 at 19:43:02, caitpotter88 wrote: > On 2016/05/13 19:28:11, Dan Ehrenberg wrote: > > ...
4 years, 7 months ago (2016-05-13 22:31:29 UTC) #33
caitp (gmail)
On 2016/05/13 22:31:29, Dan Ehrenberg wrote: > On 2016/05/13 at 19:43:02, caitpotter88 wrote: > > ...
4 years, 7 months ago (2016-05-14 02:17:55 UTC) #34
Dan Ehrenberg
On 2016/05/14 at 02:17:55, caitpotter88 wrote: > On 2016/05/13 22:31:29, Dan Ehrenberg wrote: > > ...
4 years, 7 months ago (2016-05-16 18:16:25 UTC) #35
caitp (gmail)
On 2016/05/16 18:16:25, Dan Ehrenberg wrote: > On 2016/05/14 at 02:17:55, caitpotter88 wrote: > > ...
4 years, 7 months ago (2016-05-16 18:40:21 UTC) #36
Dan Ehrenberg
On 2016/05/16 at 18:40:21, caitpotter88 wrote: > On 2016/05/16 18:16:25, Dan Ehrenberg wrote: > > ...
4 years, 7 months ago (2016-05-16 18:46:10 UTC) #37
caitp (gmail)
On 2016/05/16 18:46:10, Dan Ehrenberg wrote: > On 2016/05/16 at 18:40:21, caitpotter88 wrote: > > ...
4 years, 7 months ago (2016-05-16 18:54:31 UTC) #38
Dan Ehrenberg
On 2016/05/16 at 18:54:31, caitpotter88 wrote: > On 2016/05/16 18:46:10, Dan Ehrenberg wrote: > > ...
4 years, 7 months ago (2016-05-16 19:06:55 UTC) #39
caitp (gmail)
In the runtime patchset, there's a CHECK_NOT_NULL(generator_object_variable) --- to ensure that this is set up ...
4 years, 7 months ago (2016-05-16 19:12:53 UTC) #40
Dan Ehrenberg
As we discussed off-line, omitting the CHECK as you've already done seems to me like ...
4 years, 7 months ago (2016-05-16 20:38:37 UTC) #41
Dan Ehrenberg
https://codereview.chromium.org/1841543003/diff/650001/test/cctest/test-parsing.cc File test/cctest/test-parsing.cc (right): https://codereview.chromium.org/1841543003/diff/650001/test/cctest/test-parsing.cc#newcode7624 test/cctest/test-parsing.cc:7624: // "var O = { async method(dupe, dupe) {} ...
4 years, 7 months ago (2016-05-16 21:35:07 UTC) #42
caitp (gmail)
https://codereview.chromium.org/1841543003/diff/650001/test/cctest/test-parsing.cc File test/cctest/test-parsing.cc (right): https://codereview.chromium.org/1841543003/diff/650001/test/cctest/test-parsing.cc#newcode7624 test/cctest/test-parsing.cc:7624: // "var O = { async method(dupe, dupe) {} ...
4 years, 7 months ago (2016-05-16 21:36:19 UTC) #43
Dan Ehrenberg
lgtm
4 years, 7 months ago (2016-05-16 21:51:53 UTC) #44
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1841543003/670001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1841543003/670001
4 years, 7 months ago (2016-05-16 22:34:57 UTC) #46
commit-bot: I haz the power
Committed patchset #33 (id:670001)
4 years, 7 months ago (2016-05-16 23:17:25 UTC) #47
commit-bot: I haz the power
4 years, 7 months ago (2016-05-16 23:19:15 UTC) #49
Message was sent while issue was closed.
Patchset 33 (id:??) landed as
https://crrev.com/0d43421a228b53ecf0e3a176e65618a35b047da6
Cr-Commit-Position: refs/heads/master@{#36261}

Powered by Google App Engine
This is Rietveld 408576698