LGTM with a few small comments.
http://codereview.chromium.org/8352039/diff/8001/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/8352039/diff/8001/src/heap.cc#newcode4417 src/heap.cc:4417: maybe_result->ToObject(&result); I see what you mean, that's less useful in raw allocation functions. Still, nothing needs specifically a ScopeInfo*: FixedArray* scope_info; MaybeObject* maybe_scope_info = AllocateFixedArray(length, TENURED); if (!maybe_scope_info->To(&scope_info)) return maybe_scope_info; scope_info->set_map(scope_info_map()); return scope_info; http://codereview.chromium.org/8352039/diff/8001/src/objects.h File src/objects.h (right): http://codereview.chromium.org/8352039/diff/8001/src/objects.h#newcode3110 src/objects.h:3110: I'm pretty sure the one from Chromium's depot_tools will complain. That's the one that's run on the buildbots. http://codereview.chromium.org/8352039/diff/13001/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/8352039/diff/13001/src/runtime.cc#newcode10795 src/runtime.cc:10795: String* name = scope_info->LocalName(i); On 2011/11/02 18:52:59, Steven wrote:
Shall I keep the handle here, to be consistent in handlified code?
I think so, even though it allocates a handle per loop iteration. http://codereview.chromium.org/8352039/diff/9027/src/frames.cc File src/frames.cc (right): http://codereview.chromium.org/8352039/diff/9027/src/frames.cc#newcode1045 src/frames.cc:1045: if (i < scope_info->NumParameters()) { OK, but "Num" grinds on me in identifier names, because it reads badly. ParameterCount is only one character longer. http://codereview.chromium.org/8352039/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
