http://codereview.chromium.org/652027/diff/3033/3034
File samples/shell.cc (right):
http://codereview.chromium.org/652027/diff/3033/3034#newcode52
samples/shell.cc:52: v8::Debug::EnableAgent("shell", 9222, true);
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Please revert this. We don't want the sample shell starting the debug
agent.
I just wanted to show how I make liveedit-delay.js debuggable.
Will remove soon.
http://codereview.chromium.org/652027/diff/3033/3036
File src/compiler.cc (right):
http://codereview.chromium.org/652027/diff/3033/3036#newcode121
src/compiler.cc:121: Handle<Code> MakeCodeForLiveEdit(CompilationInfo*
info) {
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Should the decleration in compiler.h be in #ifdef
ENABLE_DEBUGGER_SUPPORT/#endif
as well?
Done.
http://codereview.chromium.org/652027/diff/3033/3038
File src/debug-delay.js (right):
http://codereview.chromium.org/652027/diff/3033/3038#newcode750
src/debug-delay.js:750: Debug.change_script_live = function(script,
change_pos, change_len, new_str, change_log) {
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Why wrap the call to LiveEditChangeScript?
Historical reasons.
Done
http://codereview.chromium.org/652027/diff/3033/3038#newcode1987
src/debug-delay.js:1987: response.failed('Script not found by id');
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Maybe remove " by id".
Done.
http://codereview.chromium.org/652027/diff/3033/3038#newcode1992
src/debug-delay.js:1992: Debug.change_script_live(the_script,
change_pos, change_len, new_string,
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Indentation of change_log.
Done.
http://codereview.chromium.org/652027/diff/3033/3039
File src/debug.cc (right):
http://codereview.chromium.org/652027/diff/3033/3039#newcode716
src/debug.cc:716: Handle<Script>
script(Script::cast(function->shared()->script()));
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Why does the liveedit.js script need to have type normal?
If so please don't match on name, but add an argument to
CompileDebuggerScript.
I just wanted to show how I make liveedit-delay.js debuggable.
Will remove soon.
http://codereview.chromium.org/652027/diff/3033/3039#newcode1966
src/debug.cc:1966: // Handle debugger actions when a new script is
compiled.
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Please don't put named TODO's in the code. If fun is not used, you
might as well
remove it.
I meant it rather as a TODISCUSS.
Done
http://codereview.chromium.org/652027/diff/3033/3040
File src/debug.h (right):
http://codereview.chromium.org/652027/diff/3033/3040#newcode609
src/debug.h:609: bool send_from_debugger);
On 2010/02/25 22:33:59, Søren Gjesse wrote:
I don't like the boolean flag, as it does not provide any information
when used.
Maybe you could add an enum as we do in other places, e.g.
Please add an enum
enum AfterCompileFlag {
NO_AFTER_COMPILE_FLAG,
SEND_WHEN_DEBUGGING
};
or maybe make a separate function for the special case of sending
compiled code
when debugging.
I added enum. Is it okay to have enum inside Debugger class?
Done
http://codereview.chromium.org/652027/diff/3033/3041
File src/liveedit-delay.js (right):
http://codereview.chromium.org/652027/diff/3033/3041#newcode31
src/liveedit-delay.js:31: /**
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Please use // style comments.
Done.
http://codereview.chromium.org/652027/diff/3033/3041#newcode47
src/liveedit-delay.js:47: Debug.LiveEditChangeScript = function
LiveEditChangeScript(script, change_pos,
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Normally the function is without name.
Done.
http://codereview.chromium.org/652027/diff/3033/3041#newcode50
src/liveedit-delay.js:50: function Assert(condition, message) {
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Indentation.
Done.
http://codereview.chromium.org/652027/diff/3033/3041#newcode97
src/liveedit-delay.js:97: function DebugGatherCompileInfo2(source) {
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Why does the name end with 2?
Comment indentation (tab?).
Done.
http://codereview.chromium.org/652027/diff/3033/3041#newcode102
src/liveedit-delay.js:102: // Sort function infos as by start position
field.
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Remove as.
Done.
http://codereview.chromium.org/652027/diff/3033/3041#newcode110
src/liveedit-delay.js:110: for (var i = 0; i < compile_info.length; i++)
{
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Will it be possible to use Array.sort here?
I have to build old_index_map in parallel. I guess Array.sort won't make
the program shorter because of this.
Of course Array.sort is N log N, but I don't know whether we really need
it now.
http://codereview.chromium.org/652027/diff/3033/3041#newcode154
src/liveedit-delay.js:154: return compile_info;
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Indentation.
Done.
http://codereview.chromium.org/652027/diff/3033/3041#newcode175
src/liveedit-delay.js:175: // the one, that comes last in this list.
On 2010/02/25 22:33:59, Søren Gjesse wrote:
comes last -> is later/is further down
Thanks
Done
http://codereview.chromium.org/652027/diff/3033/3041#newcode181
src/liveedit-delay.js:181: // Now we have stand at the last function
that begins before the change
On 2010/02/25 22:33:59, Søren Gjesse wrote:
have stand -> are
Done.
http://codereview.chromium.org/652027/diff/3033/3041#newcode183
src/liveedit-delay.js:183: // this function or enclosing one.
On 2010/02/25 22:33:59, Søren Gjesse wrote:
or enclosing -> or the enclosing
Done.
http://codereview.chromium.org/652027/diff/3033/3041#newcode191
src/liveedit-delay.js:191: // A list of function that must be dropped
from stack before changes are
On 2010/02/25 22:33:59, Søren Gjesse wrote:
from stack -> from the stack
Done.
http://codereview.chromium.org/652027/diff/3033/3041#newcode195
src/liveedit-delay.js:195: // An index of a single function, that is
going to have its Code replaced.
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Code -> code
Done.
http://codereview.chromium.org/652027/diff/3033/3044
File src/runtime.cc (right):
http://codereview.chromium.org/652027/diff/3033/3044#newcode8032
src/runtime.cc:8032: if (counter < buffer_size) {
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Please add an ASSERT for the object not in new space when skipping a
write
barrier.
Actually I changed the flag instead. Is it ok?
Done.
http://codereview.chromium.org/652027/diff/3033/3044#newcode8040
src/runtime.cc:8040: /**
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Please use // for comments.
Done.
http://codereview.chromium.org/652027/diff/3033/3044#newcode8071
src/runtime.cc:8071: * For a script calculates compilation infomation
about all its functions
On 2010/02/25 22:33:59, Søren Gjesse wrote:
calcolates -> calculate, infomation -> information
Done.
http://codereview.chromium.org/652027/diff/3033/3044#newcode8097
src/runtime.cc:8097: static Object*
Runtime_LiveEditReplaceScriptLive(Arguments args) {
On 2010/02/25 22:33:59, Søren Gjesse wrote:
These functions names both starts and ends with Live. How about
dropping the
last Live?
Sure. Totally forgot about this.
Done
http://codereview.chromium.org/652027/diff/3033/3044#newcode8124
src/runtime.cc:8124: * Replaces Code of SharedFunctionInfo with a new
one.
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Code -> code
Done.
http://codereview.chromium.org/652027/diff/3033/3044#newcode8138
src/runtime.cc:8138: * Connectes SharedFunctionInfo to another script
(representing the old
On 2010/02/25 22:33:59, Søren Gjesse wrote:
Connectes -> Connects
Done.
http://codereview.chromium.org/652027/diff/3033/3044#newcode8141
src/runtime.cc:8141: static Object*
Runtime_LiveEditLinkToOldScriptLive(Arguments args) {
On 2010/02/25 22:33:59, Søren Gjesse wrote:
I suggest remove Old from the function name, as the caller can pass
any script.
Good point. How about LiveEditRelinkFunctionToScript?
Done
http://codereview.chromium.org/652027/diff/3033/3048
File tools/js2c.py (right):
http://codereview.chromium.org/652027/diff/3033/3048#newcode295
tools/js2c.py:295: # lines = minifier.JSMinify(lines)
On 2010/02/25 22:33:59, Søren Gjesse wrote:
???
I did this to make liveedit-delay.js more debuggable.
Will remove this soon.
http://codereview.chromium.org/652027
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
