Thanks a lot for spotting this. Hopefully, I addressed your concern. The main problem I faced is how to fail if there is pending exception. I just added an
explicit check, but if there is some other method, I'd be glad to update the
test.

On 2010/02/10 13:19:41, Mads Ager wrote:
Thanks Anton, I would like to make sure that the exception throwing part of
SetPrototype works correctly.  Other than that it looks good.

http://codereview.chromium.org/598020/diff/1007/1010
File src/api.cc (right):

http://codereview.chromium.org/598020/diff/1007/1010#newcode2040
src/api.cc:2040: return !i::SetPrototype(self, value_obj).is_null();
I think we need to use the EXCEPTION_PREAMPLE and EXCEPTION_BAILOUT_CHECK
macros
here to make sure that we correctly clear exceptions? We should probably add
some tests of exception handling for this API method.

http://codereview.chromium.org/598020/diff/1007/1015
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/598020/diff/1007/1015#newcode4846
test/cctest/test-api.cc:4846: THREADED_TEST(SetPrototype) {
Could you add tests that attempt to create a cycle in the prototype chain?
That
will throw an exception and we should make sure that we can catch it with an
API
TryCatch handler and that we can execute more code after we do that (to check
that the exception flags are cleared correctly).



http://codereview.chromium.org/598020

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

Reply via email to