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

Issue 8226017: Introduce collective --harmony flag. (Closed)

Created:
9 years, 2 months ago by rossberg
Modified:
9 years, 2 months ago
Reviewers:
Steven
CC:
v8-dev
Visibility:
Public.

Description

Introduce collective --harmony flag. Shorten --harmony-block-scoping to --harmony-scoping. R=keuchel@chromium.org BUG= TEST= Committed: http://code.google.com/p/v8/source/detail?r=9589

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -74 lines) Patch
M src/api.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M src/arm/full-codegen-arm.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M src/compiler.cc View 2 chunks +3 lines, -4 lines 0 comments Download
M src/flag-definitions.h View 1 chunk +2 lines, -1 line 0 comments Download
M src/ia32/full-codegen-ia32.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M src/mips/full-codegen-mips.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M src/parser.h View 3 chunks +4 lines, -4 lines 0 comments Download
M src/parser.cc View 14 chunks +21 lines, -24 lines 0 comments Download
M src/preparser.h View 2 chunks +2 lines, -2 lines 0 comments Download
M src/preparser.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/scanner.h View 2 chunks +5 lines, -5 lines 0 comments Download
M src/scanner.cc View 4 chunks +4 lines, -4 lines 0 comments Download
M src/v8.cc View 1 chunk +9 lines, -0 lines 2 comments Download
M src/x64/full-codegen-x64.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M test/cctest/test-parsing.cc View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/bugs/harmony/debug-blockscopes.js View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/harmony/block-conflicts.js View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/harmony/block-leave.js View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/harmony/block-let-crankshaft.js View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/harmony/block-let-declaration.js View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/harmony/block-let-semantics.js View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/harmony/block-scoping.js View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/harmony/debug-blockscopes.js View 1 chunk +1 line, -1 line 0 comments Download
M test/mjsunit/harmony/debug-evaluate-blockscopes.js View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
rossberg
9 years, 2 months ago (2011-10-12 11:13:45 UTC) #1
Steven
LGTM http://codereview.chromium.org/8226017/diff/1/src/v8.cc File src/v8.cc (right): http://codereview.chromium.org/8226017/diff/1/src/v8.cc#newcode61 src/v8.cc:61: // TODO(rossberg): Is there a better place to ...
9 years, 2 months ago (2011-10-12 11:54:38 UTC) #2
rossberg
9 years, 2 months ago (2011-10-12 12:06:56 UTC) #3
http://codereview.chromium.org/8226017/diff/1/src/v8.cc
File src/v8.cc (right):

http://codereview.chromium.org/8226017/diff/1/src/v8.cc#newcode61
src/v8.cc:61: // TODO(rossberg): Is there a better place to put this?
On 2011/10/12 11:54:38, Steven wrote:
> This is indeed not such a cool place to put it. You could consider putting it
at
> the end of  FlagList::SetFlagsFromCommandLine in flags.cc or 
> doing something in Shell::SetOptions in d8.cc. At least that would group this
> code with other options/flags code.

Well, I think flags.cc would be the wrong place to put it, since that should be
generic and hence oblivious to individual flags. (It would also create a
circular dependency, I suspect.)

D8 is also the wrong place, because then it will only work for D8. ;-)

Ideally, flags.cc would provide a generic way to express implications between
flags. But that is clearly overkill at this point

Powered by Google App Engine
This is Rietveld 408576698