LGTM!
http://codereview.chromium.org/3141047/diff/1/10 File src/debug.h (right): http://codereview.chromium.org/3141047/diff/1/10#newcode332 src/debug.h:332: k_after_break_target_address, Weird naming. Should all be kAfterBreak... right? Feel free to keep it for now. http://codereview.chromium.org/3141047/diff/1/13 File src/ia32/debug-ia32.cc (right): http://codereview.chromium.org/3141047/diff/1/13#newcode116 src/ia32/debug-ia32.cc:116: __ SmiTag(reg); Is there any way you could add code that checks that we're not losing any information by the tagging process (maybe check that the top two bits of reg are 00)? Goes for all platforms. http://codereview.chromium.org/3141047/diff/1/13#newcode136 src/ia32/debug-ia32.cc:136: __ Set(reg, Immediate(1)); if (FLAG_debug_code)? Use a zap value instead? Use on the other platforms? http://codereview.chromium.org/3141047/diff/1/15 File src/ia32/macro-assembler-ia32.cc (right): http://codereview.chromium.org/3141047/diff/1/15#newcode333 src/ia32/macro-assembler-ia32.cc:333: Extra spacing here. http://codereview.chromium.org/3141047/diff/1/15#newcode353 src/ia32/macro-assembler-ia32.cc:353: Extra spacing here. http://codereview.chromium.org/3141047/diff/1/24 File test/cctest/test-debug.cc (right): http://codereview.chromium.org/3141047/diff/1/24#newcode1188 test/cctest/test-debug.cc:1188: v8::Script::Compile(v8::String::New("function foo(){return new bar(1).x;}"))->Run(); Line too long. http://codereview.chromium.org/3141047/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
