http://codereview.chromium.org/6397011/diff/1/src/api.cc File src/api.cc (right):
http://codereview.chromium.org/6397011/diff/1/src/api.cc#newcode2585 src/api.cc:2585: has_pending_exception = value->IsFailure(); On 2011/01/28 11:39:18, Mads Ager wrote:
Should you test for value->IsException instead and assert that it
cannot be
OutOfMemory or RetryAfterGC?
I can, but not sure if it's a good idea. Plus technically we can probably have OOM here if callback is JS thing which exhausts the memory. I hope that EXCEPTION_ macros should cope with any kind of failure. Am I missing something? http://codereview.chromium.org/6397011/diff/1/src/messages.cc File src/messages.cc (right): http://codereview.chromium.org/6397011/diff/1/src/messages.cc#newcode140 src/messages.cc:140: Top::clear_pending_exception(); Let me try modified second approach: I'll move all the logic of thread_local_ save and restore over here, does that look reasonable? On 2011/01/28 11:39:18, Mads Ager wrote:
Should this be moved to the caller? I think we only call this if there
is
pending exception. In the caller I think we always set the
pending_exception
back at the end. So, in the caller, maybe we should clear the pending
exception
before calling this?
If not, I think the comment should be extended to state that this is
only called
from a place where the pending exception is restored on return?
Ignoring new exceptions while report the current one seems like the
right thing
to do.
http://codereview.chromium.org/6397011/diff/1/src/top.cc File src/top.cc (right): http://codereview.chromium.org/6397011/diff/1/src/top.cc#newcode982 src/top.cc:982: On 2011/01/28 11:39:18, Mads Ager wrote:
Clear pending exception here instead of in ReportMessage? It is
restored
correctly below?
See above. http://codereview.chromium.org/6397011/diff/1/src/top.h File src/top.h (right): http://codereview.chromium.org/6397011/diff/1/src/top.h#newcode457 src/top.h:457: static void PropagatePendingExceptionToExteranlTryCatch(); On 2011/01/28 11:39:18, Mads Ager wrote:
Exteranl -> External
Done. http://codereview.chromium.org/6397011/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
