LGTM. On Fri, Oct 24, 2008 at 3:10 PM, <[EMAIL PROTECTED]> wrote: > Hello [EMAIL PROTECTED], [EMAIL PROTECTED], > > I'd like you to do a code review. To review this change, run > > gvn review --project https://v8.googlecode.com/svn [EMAIL PROTECTED]/[EMAIL > PROTECTED] > > Alternatively, to review the latest snapshot of this change > branch, run > > gvn --project https://v8.googlecode.com/svn review [EMAIL PROTECTED]/xxx > > to review the following change: > > [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-10-24 14:08:31 > +-100 (Fri, 24 Oct 2008) > > Description: > > Revorked version of http://codereview.chromium.org/8101. The only change is > that the exception stored in the TryCatch is not cleared as that was the > cause of the regression. > > So the actual difference to http://codereview.chromium.org/8101 is: > Index: src/top.h > =================================================================== > --- src/top.h (revision 576) > +++ src/top.h (working copy) > @@ -154,10 +154,6 @@ > if (has_pending_exception()) { > thread_local_.external_caught_exception_ = > thread_local_.pending_external_caught_exception_; > - } else { > - if (thread_local_.try_catch_handler_ != NULL) { > - thread_local_.try_catch_handler_->Reset(); > - } > } > thread_local_.pending_external_caught_exception_ = false; > } > @@ -312,14 +308,14 @@ > }; > > > > > Affected Paths: > M //branches/bleeding_edge/src/execution.cc > M //branches/bleeding_edge/src/top.cc > M //branches/bleeding_edge/src/top.h > > > This is a semiautomated message from "gvn mail". See > <http://code.google.com/p/gvn/> to learn more. > > Index: src/execution.cc > =================================================================== > --- src/execution.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/execution.cc (^/changes/[EMAIL > PROTECTED]/xxx/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -91,12 +91,10 @@ static Handle<Object> Invoke(bool construct, > value->Verify(); > #endif > > - // Update the pending exception flag and return the value. > + // Update the pending exception and external caught flag and return the > value. > *has_pending_exception = value->IsException(); > ASSERT(*has_pending_exception == Top::has_pending_exception()); > - if (*has_pending_exception) { > - Top::setup_external_caught(); > - } > + Top::propagate_external_caught(); > > // If the pending exception is OutOfMemoryException set out_of_memory in > // the global context. Note: We have to mark the global context here > Index: src/top.cc > =================================================================== > --- src/top.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/top.cc (^/changes/[EMAIL PROTECTED]/xxx/bleeding_edge/src/[EMAIL > PROTECTED]) > @@ -101,7 +101,7 @@ void Top::InitializeThreadLocal() { > clear_pending_exception(); > clear_scheduled_exception(); > thread_local_.save_context_ = NULL; > - thread_local_.catcher_ = NULL; > + thread_local_.pending_external_caught_exception_ = false; > } > > > @@ -607,7 +607,7 @@ Failure* Top::Throw(Object* exception, MessageLoca > > > Failure* Top::ReThrow(Object* exception, MessageLocation* location) { > - // Set the exception beeing re-thrown. > + // Set the exception being re-thrown. > set_pending_exception(exception); > return Failure::Exception(); > } > @@ -789,9 +789,8 @@ void Top::DoThrow(Object* exception, > // If the exception is caught externally, we store it in the > // try/catch handler. The C code can find it later and process it if > // necessary. > - thread_local_.catcher_ = NULL; > + thread_local_.pending_external_caught_exception_ = is_caught_externally; > if (is_caught_externally) { > - thread_local_.catcher_ = thread_local_.try_catch_handler_; > thread_local_.try_catch_handler_->exception_ = > reinterpret_cast<void*>(*exception_handle); > if (!message_obj.is_null()) { > Index: src/top.h > =================================================================== > --- src/top.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/top.h (^/changes/[EMAIL PROTECTED]/xxx/bleeding_edge/src/[EMAIL > PROTECTED]) > @@ -53,7 +53,9 @@ class ThreadLocalTop BASE_EMBEDDED { > bool external_caught_exception_; > v8::TryCatch* try_catch_handler_; > SaveContext* save_context_; > - v8::TryCatch* catcher_; > + // Flag indicating that the try_catch_handler_ contains an exception to be > + // caught. > + bool pending_external_caught_exception_; > > // Stack. > Address c_entry_fp_; // the frame pointer of the top c entry frame > @@ -144,10 +146,16 @@ class Top { > thread_local_.scheduled_exception_ = Heap::the_hole_value(); > } > > - static void setup_external_caught() { > - thread_local_.external_caught_exception_ = > - (thread_local_.catcher_ != NULL) && > - (Top::thread_local_.try_catch_handler_ == > Top::thread_local_.catcher_); > + // Propagate the external caught exception flag. If there is no external > + // caught exception always clear the TryCatch handler as it might contain > + // an exception object from a throw which was bypassed by a finally block > + // doing an explicit return/break/continue. > + static void propagate_external_caught() { > + if (has_pending_exception()) { > + thread_local_.external_caught_exception_ = > + thread_local_.pending_external_caught_exception_; > + } > + thread_local_.pending_external_caught_exception_ = false; > } > > // Tells whether the current context has experienced an out of memory > >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
