Mostly only responded to comments I didn't agree with entirely.
http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js File test/mjsunit/arguments-read-and-assignment.js (right): http://codereview.chromium.org/8888006/diff/1/test/mjsunit/arguments-read-and-assignment.js#newcode74 test/mjsunit/arguments-read-and-assignment.js:74: Feel free. I'll try not to expand the scope of this CL too much. Also, it's not just functions, but function applications. It's hard enough to read as is. http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-concat.js File test/mjsunit/array-concat.js (right): http://codereview.chromium.org/8888006/diff/1/test/mjsunit/array-concat.js#newcode228 test/mjsunit/array-concat.js:228: function mkGetter(i) { return function() { trace.push(i); }; } No, it ends the return statement. http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js File test/mjsunit/debug-backtrace.js (right): http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode126 test/mjsunit/debug-backtrace.js:126: json = '{"seq":0,"type":"request","command":"backtrace","arguments":{"fromFrame":1,"toFrame":3}}'; Single string JSON literal. I don't want to start splitting it (and change it from a sequential string to a cons string). http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode144 test/mjsunit/debug-backtrace.js:144: json = '{"seq":0,"type":"request","command":"backtrace","arguments":{"fromFrame":0,"toFrame":2, "bottom":true}}'; ditto. http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode176 test/mjsunit/debug-backtrace.js:176: json = '{"seq":0,"type":"request","command":"frame","arguments":{"number":0}}'; ditto. http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode190 test/mjsunit/debug-backtrace.js:190: json = '{"seq":0,"type":"request","command":"frame","arguments":{"number":1}}'; ditto http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-backtrace.js#newcode201 test/mjsunit/debug-backtrace.js:201: json = '{"seq":0,"type":"request","command":"frame","arguments":{"number":3}}'; ditto http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-breakpoints.js File test/mjsunit/debug-breakpoints.js (right): http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-breakpoints.js#newcode32 test/mjsunit/debug-breakpoints.js:32: function f() {a=1;b=2;} Actual function body is being used in tests below. Better leave it be. http://codereview.chromium.org/8888006/diff/1/test/mjsunit/debug-breakpoints.js#newcode37 test/mjsunit/debug-breakpoints.js:37: Again the missing space is because it refers to the actual body above. I'm not about to rewrite the the checks of the test in this CL. http://codereview.chromium.org/8888006/diff/1/test/mjsunit/html-comments.js File test/mjsunit/html-comments.js (right): http://codereview.chromium.org/8888006/diff/1/test/mjsunit/html-comments.js#newcode47 test/mjsunit/html-comments.js:47: var x = 1; x <! x--; Actually, it shouldn't be above either. This test checks that <!-- begins a comment, and isn't an operator, so it relies on ASI. http://codereview.chromium.org/8888006/diff/1/test/mjsunit/no-semicolon.js File test/mjsunit/no-semicolon.js (right): http://codereview.chromium.org/8888006/diff/1/test/mjsunit/no-semicolon.js#newcode30 test/mjsunit/no-semicolon.js:30: Whoops, agree. http://codereview.chromium.org/8888006/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
