LGTM, but maybe you should get Lars' feedback too?
http://codereview.chromium.org/149324/diff/1/2 File src/heap.cc (right): http://codereview.chromium.org/149324/diff/1/2#newcode1063 Line 1063: roots_[kMetaMapRootIndex] = new_meta_map; How about automagically generating a private setter for all these? http://codereview.chromium.org/149324/diff/1/11 File src/heap.h (right): http://codereview.chromium.org/149324/diff/1/11#newcode38 Line 38: V(Map, meta_map, MetaMap) \ Is it really worth giving everything two names to comply with the style guide for the index constant? I think I'd prefer just having one name... http://codereview.chromium.org/149324/diff/1/11#newcode137 Line 137: V(Object, symbol_table, SymbolTable) Why isn't this of type SymbolTable*? http://codereview.chromium.org/149324/diff/1/11#newcode918 Line 918: kRootListLength}; Put }; on a new line. http://codereview.chromium.org/149324/diff/1/10 File src/mark-compact.cc (right): http://codereview.chromium.org/149324/diff/1/10#newcode227 Line 227: if (reinterpret_cast<String*>(second) != Why is the reinterpret_cast necessary here? It's really an Object* identity check, right? It should fit nicely on a single line. http://codereview.chromium.org/149324/diff/1/10#newcode597 Line 597: reinterpret_cast<SymbolTable*>(Heap::raw_unchecked_symbol_table()); Doesn't raw_unchecked_symbol_table return something of type SymbolTable* already? Can't you get rid of reinterpret_cast? http://codereview.chromium.org/149324/diff/1/10#newcode790 Line 790: reinterpret_cast<SymbolTable*>(Heap::raw_unchecked_symbol_table()); Do you need reinterpret_cast here? http://codereview.chromium.org/149324 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
