LGTM, consider making a local TryCatch

http://codereview.chromium.org/8898021/diff/1/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/8898021/diff/1/src/d8.cc#newcode299
src/d8.cc:299: if (args[0]->IsUint32()) {
Below: How about just having a local TryCatch? The functionality in
isolate.cc should throw to the TryCatch nearest to the top of the stack

http://codereview.chromium.org/8898021/diff/1/src/d8.cc#newcode316
src/d8.cc:316: return ThrowException(String::New("Array length must be a
number."));
we already established that length is a number, so this exception seems
wrong. Can we ever end up here?

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-crbug-100859.js
File test/mjsunit/regress/regress-crbug-100859.js (right):

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-crbug-100859.js#newcode33
test/mjsunit/regress/regress-crbug-100859.js:33: exception = false;
var exeception

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-crbug-100859.js#newcode41
test/mjsunit/regress/regress-crbug-100859.js:41: assertTrue(exception);
we don't really need this: exception=false => we should have hit
assertUnreachable

http://codereview.chromium.org/8898021/diff/1/test/mjsunit/regress/regress-crbug-100859.js#newcode48
test/mjsunit/regress/regress-crbug-100859.js:48: var exception = false;
not var

http://codereview.chromium.org/8898021/

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

Reply via email to