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

Issue 9352013: Extend scanner with new Harmony module keywords (under flag). (Closed)

Created:
8 years, 10 months ago by rossberg
Modified:
8 years, 10 months ago
Reviewers:
Michael Starzinger
CC:
v8-dev
Visibility:
Public.

Description

Extend scanner with new Harmony module keywords (under flag). R=mstarzinger@chromium.org BUG= TEST= Committed: https://code.google.com/p/v8/source/detail?r=10638

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed Michael's comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -16 lines) Patch
M src/flag-definitions.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/parser.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/parser.cc View 1 3 chunks +9 lines, -1 line 0 comments Download
M src/preparser.h View 4 chunks +7 lines, -3 lines 0 comments Download
M src/scanner.h View 1 3 chunks +14 lines, -6 lines 0 comments Download
M src/scanner.cc View 5 chunks +13 lines, -5 lines 0 comments Download
M src/token.h View 1 chunk +3 lines, -0 lines 0 comments Download
M test/cctest/test-parsing.cc View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
rossberg
8 years, 10 months ago (2012-02-07 10:50:48 UTC) #1
Michael Starzinger
LGTM (with nits). https://chromiumcodereview.appspot.com/9352013/diff/1/src/scanner.h File src/scanner.h (right): https://chromiumcodereview.appspot.com/9352013/diff/1/src/scanner.h#newcode56 src/scanner.h:56: kAllowModules = 16 Can we either ...
8 years, 10 months ago (2012-02-07 14:50:59 UTC) #2
rossberg
8 years, 10 months ago (2012-02-08 10:32:42 UTC) #3
https://chromiumcodereview.appspot.com/9352013/diff/1/src/scanner.h
File src/scanner.h (right):

https://chromiumcodereview.appspot.com/9352013/diff/1/src/scanner.h#newcode56
src/scanner.h:56: kAllowModules = 16
On 2012/02/07 14:50:59, Michael Starzinger wrote:
> Can we either change this constant (and the two above) to "1 << 4" or to
"0x10"?
> I think that's more readable.

Done.

https://chromiumcodereview.appspot.com/9352013/diff/1/src/scanner.h#newcode564
src/scanner.h:564: // Whether we scan 'module', 'import', 'export', and 'from'
as keywords.
On 2012/02/07 14:50:59, Michael Starzinger wrote:
> I don't see "from" being a keyword in the scanner.

Oops, leftover.

Powered by Google App Engine
This is Rietveld 408576698