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

Issue 2091313002: [parser] don't report error for CoverInitializedNames in async arrow formals (Closed)

Created:
4 years, 6 months ago by caitp (gmail)
Modified:
4 years, 6 months ago
CC:
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

[parser] don't report error for CoverInitializedNames in async arrow formals BUG=v8:4483, v8:5148 R=littledan@chromium.org, adamk@chromium.org, jwolfe@igalia.com, nikolaos@chromium.org Committed: https://crrev.com/4bb1f70e66ef361f42fdb6082cd4db9def77befc Cr-Commit-Position: refs/heads/master@{#37260}

Patch Set 1 #

Total comments: 8

Patch Set 2 : nits #

Patch Set 3 : [parser] don't report error for CoverInitializedNames in async arrow formals #

Total comments: 1

Patch Set 4 : Fix compile error #

Patch Set 5 : _Really_ fix compile error #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -8 lines) Patch
M src/parsing/parser-base.h View 1 2 chunks +15 lines, -6 lines 0 comments Download
M test/cctest/test-parsing.cc View 1 2 3 4 4 chunks +34 lines, -0 lines 0 comments Download
M test/mjsunit/harmony/async-await-basic.js View 1 1 chunk +14 lines, -2 lines 0 comments Download

Messages

Total messages: 25 (8 generated)
caitp (gmail)
PTAL, a fix for the bug I reported last night (re: CoverInitializedNames in async arrow ...
4 years, 6 months ago (2016-06-24 14:29:38 UTC) #1
Dan Ehrenberg
https://codereview.chromium.org/2091313002/diff/1/src/parsing/parser-base.h File src/parsing/parser-base.h (right): https://codereview.chromium.org/2091313002/diff/1/src/parsing/parser-base.h#newcode2203 src/parsing/parser-base.h:2203: if ((!maybe_arrow || peek() != Token::ARROW)) { Nit: remove ...
4 years, 6 months ago (2016-06-24 17:30:41 UTC) #2
caitp (gmail)
https://codereview.chromium.org/2091313002/diff/1/src/parsing/parser-base.h File src/parsing/parser-base.h (right): https://codereview.chromium.org/2091313002/diff/1/src/parsing/parser-base.h#newcode2203 src/parsing/parser-base.h:2203: if ((!maybe_arrow || peek() != Token::ARROW)) { On 2016/06/24 ...
4 years, 6 months ago (2016-06-24 18:05:40 UTC) #3
Dan Ehrenberg
https://codereview.chromium.org/2091313002/diff/1/test/cctest/test-parsing.cc File test/cctest/test-parsing.cc (right): https://codereview.chromium.org/2091313002/diff/1/test/cctest/test-parsing.cc#newcode7674 test/cctest/test-parsing.cc:7674: "async({ foo = 1 })", On 2016/06/24 at 18:05:40, ...
4 years, 6 months ago (2016-06-24 18:30:14 UTC) #4
caitp (gmail)
https://codereview.chromium.org/2091313002/diff/1/test/cctest/test-parsing.cc File test/cctest/test-parsing.cc (right): https://codereview.chromium.org/2091313002/diff/1/test/cctest/test-parsing.cc#newcode7674 test/cctest/test-parsing.cc:7674: "async({ foo = 1 })", On 2016/06/24 18:30:14, Dan ...
4 years, 6 months ago (2016-06-24 19:24:43 UTC) #5
Dan Ehrenberg
Thanks for the additional tests, but they seem to show that the patch is not ...
4 years, 6 months ago (2016-06-24 21:31:00 UTC) #6
caitp (gmail)
On 2016/06/24 21:31:00, Dan Ehrenberg wrote: > Thanks for the additional tests, but they seem ...
4 years, 6 months ago (2016-06-24 21:33:44 UTC) #7
Dan Ehrenberg
lgtm Sorry for my confusion, of course ({foo = 1}) is a SyntaxError. This patch ...
4 years, 6 months ago (2016-06-24 22:31:30 UTC) #8
caitp (gmail)
On 2016/06/24 22:31:30, Dan Ehrenberg wrote: > lgtm > > Sorry for my confusion, of ...
4 years, 6 months ago (2016-06-24 22:52:28 UTC) #9
caitp (gmail)
On 2016/06/24 22:31:30, Dan Ehrenberg wrote: > lgtm > > Sorry for my confusion, of ...
4 years, 6 months ago (2016-06-24 22:52:29 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2091313002/40001
4 years, 6 months ago (2016-06-24 23:57:57 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/builds/18274) v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED, ...
4 years, 6 months ago (2016-06-25 00:02:02 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2091313002/60001
4 years, 6 months ago (2016-06-25 00:06:04 UTC) #17
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/builds/18275) v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED, ...
4 years, 6 months ago (2016-06-25 00:08:12 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2091313002/80001
4 years, 6 months ago (2016-06-25 00:13:20 UTC) #22
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 6 months ago (2016-06-25 00:38:28 UTC) #23
commit-bot: I haz the power
4 years, 6 months ago (2016-06-25 00:39:20 UTC) #25
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/4bb1f70e66ef361f42fdb6082cd4db9def77befc
Cr-Commit-Position: refs/heads/master@{#37260}

Powered by Google App Engine
This is Rietveld 408576698