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

Issue 1262913003: [es6] Remove Scanner and Parser flags for harmony_modules (Closed)

Created:
5 years, 4 months ago by adamk
Modified:
5 years, 4 months ago
Reviewers:
rossberg
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[es6] Remove Scanner and Parser flags for harmony_modules These flags weren't doing any real work, since the decision of whether some source code is a script or module is made outside the parser (currently, by the V8 API). The only behavior change in this patch is to always parse 'import' and 'export' as their Token values, which changes the error message from "Unexpected reserved word" to "Unexpected token import" (which doesn't seem particularly harmful). Committed: https://crrev.com/5c34bacb727e031da0dad96d0a3ee55df40a09b9 Cr-Commit-Position: refs/heads/master@{#30034}

Patch Set 1 #

Patch Set 2 : Remove call to SetHarmonyModules from test-parsing #

Patch Set 3 : Add flag-setting to cctests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -94 lines) Patch
M src/parser.cc View 3 chunks +0 lines, -3 lines 0 comments Download
M src/preparser.h View 2 chunks +1 line, -3 lines 0 comments Download
M src/scanner.h View 2 chunks +0 lines, -9 lines 0 comments Download
M src/scanner.cc View 5 chunks +64 lines, -71 lines 0 comments Download
M test/cctest/test-parsing.cc View 1 2 9 chunks +4 lines, -8 lines 0 comments Download

Messages

Total messages: 13 (6 generated)
adamk
5 years, 4 months ago (2015-08-05 01:03:55 UTC) #2
rossberg
LGTM, modulo build problem
5 years, 4 months ago (2015-08-05 08:02:40 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1262913003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1262913003/20001
5 years, 4 months ago (2015-08-05 15:55:42 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux64_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel/builds/8406) v8_mac_rel on tryserver.v8 (JOB_FAILED, ...
5 years, 4 months ago (2015-08-05 16:08:34 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1262913003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1262913003/40001
5 years, 4 months ago (2015-08-05 17:13:11 UTC) #11
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 4 months ago (2015-08-05 17:59:44 UTC) #12
commit-bot: I haz the power
5 years, 4 months ago (2015-08-05 18:00:15 UTC) #13
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/5c34bacb727e031da0dad96d0a3ee55df40a09b9
Cr-Commit-Position: refs/heads/master@{#30034}

Powered by Google App Engine
This is Rietveld 408576698