Correct propagation of exceptions from setters.

Correct propagation of exceptions from setters.


BUG=v8:1105
TEST=test/mjsunit/regress/regress-1105.js

Committed: http://code.google.com/p/v8/source/detail?r=6680

[antonm, 2011-02-08 13:33:01]

LGTM

http://codereview.chromium.org/6451003/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/6451003/diff/1/src/runtime.cc#newcode1093
src/runtime.cc:1093: if (result.is_null()) return Failure::Exception();
I know we do not typically do it, so feel free to ignore, but maybe we should
add something like ASSERT(Top::has_pending_exception()) on bailouts and
ASSERT(!Top::has_pending_exception()) on normal path.

But again, we don't typically do it.

http://codereview.chromium.org/6451003/diff/1/test/mjsunit/regress/regress-11...
File test/mjsunit/regress/regress-1105.js (right):

http://codereview.chromium.org/6451003/diff/1/test/mjsunit/regress/regress-11...
test/mjsunit/regress/regress-1105.js:31:
this.eval('__defineSetter__("x",function(){throw 42;});')
why eval here?  Couldn't it be plain this.__defineSetter__ ?

[Lasse Reichstein, 2011-02-08 13:38:56]

http://codereview.chromium.org/6451003/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/6451003/diff/1/src/runtime.cc#newcode1093
src/runtime.cc:1093: if (result.is_null()) return Failure::Exception();
And yet, if we had done it here, we would have caught this much faster.
I'll add them.

http://codereview.chromium.org/6451003/diff/1/test/mjsunit/regress/regress-11...
File test/mjsunit/regress/regress-1105.js (right):

http://codereview.chromium.org/6451003/diff/1/test/mjsunit/regress/regress-11...
test/mjsunit/regress/regress-1105.js:31:
this.eval('__defineSetter__("x",function(){throw 42;});')
Ah, that works too.
This was a blind copy of the original example, using eval instead of load.
Thanks.