LGTM

It would be nice with a bit of extra cleanup in the scope info code.


http://codereview.chromium.org/2908009/diff/1/15
File src/objects-inl.h (right):

http://codereview.chromium.org/2908009/diff/1/15#newcode2650
src/objects-inl.h:2650: ScopeInfoObject*
SharedFunctionInfo::scope_info() {
Why can't you use

ACCESSORS(SharedFunctionInfo, scope_info, ScopeInfoObject,
kScopeInfoOffset)?

http://codereview.chromium.org/2908009/diff/1/16
File src/scopeinfo.cc (right):

http://codereview.chromium.org/2908009/diff/1/16#newcode332
src/scopeinfo.cc:332: // For variables allocated in the context they are
always preceded by the
by the number Context... number of fixed allocated
->
by Context... fixed allocated

http://codereview.chromium.org/2908009/diff/1/16#newcode368
src/scopeinfo.cc:368: // +2 for function name and calls eval:
: -> .

http://codereview.chromium.org/2908009/diff/1/16#newcode376
src/scopeinfo.cc:376: int n;  // number of context slots;
Call this number_of_context_slots instead of having a comment?

http://codereview.chromium.org/2908009/diff/1/16#newcode385
src/scopeinfo.cc:385: int n;  // number of parameter slots;
Call the variable number_of_parameter_slots instead of the comment?

http://codereview.chromium.org/2908009/diff/1/16#newcode393
src/scopeinfo.cc:393: // +1 for function name:
: -> .

http://codereview.chromium.org/2908009/diff/1/16#newcode406
src/scopeinfo.cc:406: int n;  // number of stack slots;
Argh. I know this is not your changes at all. But could you rename these
variables instead of having to need the comments. That would be a nice
cleanup and there are more below.

http://codereview.chromium.org/2908009/diff/1/16#newcode458
src/scopeinfo.cc:458: // slots start after length entry
Capitalize comment and end with period.

http://codereview.chromium.org/2908009/diff/1/16#newcode462
src/scopeinfo.cc:462: Object** pn = p0 + n * 2;
Better names for variables would be nice. pn -> end or
last_context_entry or something.

http://codereview.chromium.org/2908009/show

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

Reply via email to