http://codereview.chromium.org/12901/diff/1/5 File src/builtins.cc (right):
http://codereview.chromium.org/12901/diff/1/5#newcode389 Line 389: Top::ReportPendingMessages(); On 2008/12/03 15:44:48, Mads Ager wrote: > I'm not sure I understand why we need to check here? There are a lot of places > in the code where we invoke a c++ callback function. If we need to check here, > don't we need to check in the other places as well? This was a remnant from an earlier effort to get the tests working. Removed. http://codereview.chromium.org/12901/diff/1/6 File src/execution.cc (right): http://codereview.chromium.org/12901/diff/1/6#newcode101 Line 101: Top::clear_pending_message(); On 2008/12/04 07:53:53, Søren Gjesse wrote: > If there is no pending exception clearing it should not be needed. Removed. http://codereview.chromium.org/12901/diff/1/6#newcode102 Line 102: Top::setup_external_caught(); On 2008/12/03 15:44:48, Mads Ager wrote: > Why is setup_external_caught needed in the case that there was no exception? It isn't. Removed. http://codereview.chromium.org/12901/diff/1/3 File src/top.cc (right): http://codereview.chromium.org/12901/diff/1/3#newcode742 Line 742: bool Top::IsUncaughtException(bool* is_caught_externally) { On 2008/12/04 07:53:53, Søren Gjesse wrote: > This function does not only check whether the exception is uncaught as if it is > caught by a C++ TryCatch it also checks whether that handler is verbose. This > should be taken into account with the name change (perhaps adding another > reference parameter) and the comments should also be changed as they still > mention reporting the exception. Reverting to the original name. http://codereview.chromium.org/12901/diff/1/3#newcode838 Line 838: // NOTE: Notifying the debugger may have caused new exceptions. On 2008/12/04 07:53:53, Søren Gjesse wrote: > Generating the message without reporting it can also cause a new exception. Updated the comment. http://codereview.chromium.org/12901/diff/1/3#newcode859 Line 859: = thread_local_.pending_exception_; On 2008/12/03 15:44:48, Mads Ager wrote: > We normally have the '=' on the previous line. Fixed. http://codereview.chromium.org/12901/diff/1/2 File test/cctest/test-api.cc (right): http://codereview.chromium.org/12901/diff/1/2#newcode2991 Line 2991: TEST(TryCatchFinallyUsingMessaging) { On 2008/12/04 07:53:53, Søren Gjesse wrote: > I suggest to change this test into one or more message test written in > JavaScript. We already have most of the try-catch-finally covered by a set of > message tests, with exception cancellation by returning from finally missing. Fixed. http://codereview.chromium.org/12901/diff/1/2#newcode3000 Line 3000: Script::Compile(v8_str("(function() { try { throw ''; } finally { return; } })()"))->Run(); On 2008/12/03 15:44:48, Mads Ager wrote: > These two lines seem too long? Break them by using the C++ concatenation of > adjacent strings? Fixed http://codereview.chromium.org/12901 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
