LGTM with some comments.
https://codereview.chromium.org/11412310/diff/2001/src/runtime.cc File src/runtime.cc (right): https://codereview.chromium.org/11412310/diff/2001/src/runtime.cc#newcode10858 src/runtime.cc:10858: Isolate* isolate, I'd prefer having the first argument in the same line as function name, and align the remaining ones to the first argument. https://codereview.chromium.org/11412310/diff/2001/src/runtime.cc#newcode10880 src/runtime.cc:10880: Isolate* isolate, Ditto about argument formatting. https://codereview.chromium.org/11412310/diff/2001/src/runtime.cc#newcode10914 src/runtime.cc:10914: if (SetContextLocalValue(isolate, scope_info, function_context, could you put all arguments on the second line? https://codereview.chromium.org/11412310/diff/2001/test/mjsunit/debug-set-variable-value.js File test/mjsunit/debug-set-variable-value.js (right): https://codereview.chromium.org/11412310/diff/2001/test/mjsunit/debug-set-variable-value.js#newcode128 test/mjsunit/debug-set-variable-value.js:128: ClosureTestCase.prototype.run_and_catch_ = function(runnable) { Why does this end with an underscore? https://codereview.chromium.org/11412310/diff/2001/test/mjsunit/debug-set-variable-value.js#newcode132 test/mjsunit/debug-set-variable-value.js:132: try { you could use assertThrows(function() { runnable(); }); here. https://codereview.chromium.org/11412310/diff/2001/test/mjsunit/debug-set-variable-value.js#newcode282 test/mjsunit/debug-set-variable-value.js:282: stray edit? https://codereview.chromium.org/11412310/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
