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

Issue 1433743005: [cleanup] Remove un-scoped ParseBlock from Parser (Closed)

Created:
5 years, 1 month ago by adamk
Modified:
5 years, 1 month ago
Reviewers:
Dan Ehrenberg, rossberg
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

[cleanup] Remove un-scoped ParseBlock from Parser Because the Scope will be optimized away by the call to FinalizeBlockScope in the case where there are no lexical declarations in the block, this should have no effect on anything downstream from the Parser, and simply removes duplicate parsing code. Due to the change from ParseStatement to ParseStatementListItem, this will result in slightly different error messages for lexical declarations in sloppy mode (until those are shipped). R=littledan@chromium.org, rossberg@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel Committed: https://crrev.com/b0b97da506fb26a2de553bcabccfa8c2512c0e39 Cr-Commit-Position: refs/heads/master@{#31966}

Patch Set 1 #

Patch Set 2 : Change preparser to match #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -42 lines) Patch
M src/parser.h View 1 chunk +0 lines, -3 lines 0 comments Download
M src/parser.cc View 3 chunks +2 lines, -30 lines 0 comments Download
M src/preparser.cc View 1 1 chunk +2 lines, -9 lines 0 comments Download

Messages

Total messages: 14 (5 generated)
adamk
5 years, 1 month ago (2015-11-10 19:34:50 UTC) #1
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1433743005/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1433743005/1
5 years, 1 month ago (2015-11-10 19:35:10 UTC) #3
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_mac_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/11701)
5 years, 1 month ago (2015-11-10 19:47:05 UTC) #5
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1433743005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1433743005/20001
5 years, 1 month ago (2015-11-10 19:55:23 UTC) #7
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 1 month ago (2015-11-10 21:54:01 UTC) #9
rossberg
LGTM. I suppose the separation was more a premature optimisation than anything else.
5 years, 1 month ago (2015-11-12 16:39:29 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1433743005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1433743005/20001
5 years, 1 month ago (2015-11-12 16:40:30 UTC) #12
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 1 month ago (2015-11-12 17:41:34 UTC) #13
commit-bot: I haz the power
5 years, 1 month ago (2015-11-14 23:20:55 UTC) #14
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/b0b97da506fb26a2de553bcabccfa8c2512c0e39
Cr-Commit-Position: refs/heads/master@{#31966}

Powered by Google App Engine
This is Rietveld 408576698