lgtm with some suggestions

I like the way that step in is propagates through the natives code.


http://codereview.chromium.org/10078014/diff/8001/src/array.js
File src/array.js (right):

http://codereview.chromium.org/10078014/diff/8001/src/array.js#newcode1030
src/array.js:1030: if (%DebugStepIntoBuiltinCallback(f)) {
Maybe rename DebugStepIntoBuiltinCallback to something like
DebugDoesFunctionSupportStepping.

http://codereview.chromium.org/10078014/diff/8001/src/array.js#newcode1030
src/array.js:1030: if (%DebugStepIntoBuiltinCallback(f)) {
The only thing I don't like here is the code duplication. I don't have
an immediate idea, but it would be great if we could get rid of it
somehow. It will be prone to errors when changing the code missing one
of the places.

http://codereview.chromium.org/10078014/diff/8001/src/array.js#newcode1030
src/array.js:1030: if (%DebugStepIntoBuiltinCallback(f)) {
An important thing performance wise is that it should be quite simple to
write %DebugStepIntoBuiltinCallback as a %_DebugStepIntoBuiltinCallback
(inline assembly) function if this runtime call gives performance
problems.

http://codereview.chromium.org/10078014/diff/8001/src/array.js#newcode1035
src/array.js:1035: %PrepareBreakSlotsForCallback(f);
I think the name here i somewhat misleading. Maybe
"%DebugPrepareStepInIfStepping".

http://codereview.chromium.org/10078014/diff/8001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/10078014/diff/8001/src/runtime.cc#newcode4712
src/runtime.cc:4712: // Set break slots for the callback function that
is passed to a buil-tin
"break slots" -> "one shot breakpoints"

http://codereview.chromium.org/10078014/

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

Reply via email to