http://codereview.chromium.org/10443114/diff/1/src/hashmap.h
File src/hashmap.h (right):

http://codereview.chromium.org/10443114/diff/1/src/hashmap.h#newcode310
src/hashmap.h:310:
nit: remove stray extra line

http://codereview.chromium.org/10443114/diff/3002/src/ia32/lithium-ia32.h
File src/ia32/lithium-ia32.h (right):

http://codereview.chromium.org/10443114/diff/3002/src/ia32/lithium-ia32.h#newcode2377
src/ia32/lithium-ia32.h:2377: zone_(zone),
You should explicitly store the zone in the graph and get it from
graph->zone() rather than passing it in here.

http://codereview.chromium.org/10443114/diff/3002/src/parser.cc
File src/parser.cc (right):

http://codereview.chromium.org/10443114/diff/3002/src/parser.cc#newcode5276
src/parser.cc:5276: captures_started(), zone());
nit: strange indentation

http://codereview.chromium.org/10443114/diff/3002/src/parser.cc#newcode6028
src/parser.cc:6028: Parser parser(script, parsing_flags, NULL, NULL,
info->isolate()->zone());
Seem's you don't want to use the isolate here. How about putting the
zone in the CompilationInfo?

http://codereview.chromium.org/10443114/diff/3002/src/parser.cc#newcode6037
src/parser.cc:6037: info->isolate()->zone());
Same here.

http://codereview.chromium.org/10443114/diff/3002/src/splay-tree.h
File src/splay-tree.h (left):

http://codereview.chromium.org/10443114/diff/3002/src/splay-tree.h#oldcode197
src/splay-tree.h:197:
nit: add empty line back

http://codereview.chromium.org/10443114/diff/3002/src/x64/lithium-codegen-x64.h
File src/x64/lithium-codegen-x64.h (right):

http://codereview.chromium.org/10443114/diff/3002/src/x64/lithium-codegen-x64.h#newcode48
src/x64/lithium-codegen-x64.h:48: LCodeGen(LChunk* chunk,
MacroAssembler* assembler, CompilationInfo* info,
Perhaps it's better to put the zone in the macro-assembler? That's used
used in several places.

http://codereview.chromium.org/10443114/diff/3002/src/x64/lithium-x64.h
File src/x64/lithium-x64.h (right):

http://codereview.chromium.org/10443114/diff/3002/src/x64/lithium-x64.h#newcode2235
src/x64/lithium-x64.h:2235: zone_(zone),
Put the zone in the graph, get it from there.

http://codereview.chromium.org/10443114/diff/3002/src/zone-inl.h
File src/zone-inl.h (right):

http://codereview.chromium.org/10443114/diff/3002/src/zone-inl.h#newcode109
src/zone-inl.h:109: if (zone_)
nit: always use {}, even if the block is only one line

http://codereview.chromium.org/10443114/diff/3002/src/zone-inl.h#newcode112
src/zone-inl.h:112: return ZONE->New(size);
same here.

http://codereview.chromium.org/10443114/diff/3002/src/zone.h
File src/zone.h (right):

http://codereview.chromium.org/10443114/diff/3002/src/zone.h#newcode155
src/zone.h:155: Zone* zone_;
Put this back where it was later in the file. to make diffs easier.

http://codereview.chromium.org/10443114/diff/3002/src/zone.h#newcode202
src/zone.h:202: // We override almost all List's public functions here
so that the
It's not really overriding. You're adding convenience function wrapper.

http://codereview.chromium.org/10443114/diff/3002/test/cctest/test-list.cc
File test/cctest/test-list.cc (right):

http://codereview.chromium.org/10443114/diff/3002/test/cctest/test-list.cc#newcode36
test/cctest/test-list.cc:36: struct ZeroingAllocationPolicy {
This is an unrelated, unnecessary change. Please remove it.

http://codereview.chromium.org/10443114/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to