LGTM.

http://codereview.chromium.org/42600/diff/1/2
File src/frames.cc (right):

http://codereview.chromium.org/42600/diff/1/2#newcode234
Line 234: // see EntryFrame::GetCallerState
Capitalize see (see -> See) and terminate comment with . Maybe consider
explicitly describing that this is customized versions of GetCallerState
for entry frames and GetCallerStackPointer for argument adaptor frames
and explain why it it breaks without these guards.

http://codereview.chromium.org/42600/diff/1/2#newcode235
Line 235: if (!IsValidStackAddress(
Compute the stack address (caller_fp?) in a local variable before the
IsValidStackAddress check.

http://codereview.chromium.org/42600/diff/1/2#newcode241
Line 241: // see ArgumentsAdaptorFrame::GetCallerStackPointer
I think this comment should be improved like above. It's not super clear
why you need this.

http://codereview.chromium.org/42600/diff/1/2#newcode242
Line 242: if (!reinterpret_cast<ArgumentsAdaptorFrame*>(frame)->
I would break this by introducing a local variable for the adaptor_frame
(after the cast) to make it easier to read. Maybe also document what
GetExpression(0) is by introducing a local variable (with a name like
number_of_arguments) for that?

http://codereview.chromium.org/42600

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

Reply via email to