LGTM. On Wed, Sep 10, 2008 at 4:51 PM, <[EMAIL PROTECTED]> wrote: > 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]/reenable-most-message-capturing > > to review the following change: > > [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-09-10 15:46:50 > +-100 (Wed, 10 Sep 2008) > > Description: > > Added option for TryCatches to not capture the message object on > exceptions. > > It turned out that the stack overflow fix from before had disabled > message storing in another test. Previously, stack overflows would > actually cause a message object to start being created but cause > another exception which would not be reported and that's what stopped > the infinite regress. This change resores that behavior. > > > > > Affected Paths: > M //branches/bleeding_edge/include/v8.h > M //branches/bleeding_edge/src/api.cc > M //branches/bleeding_edge/src/execution.cc > M //branches/bleeding_edge/src/top.cc > M //branches/bleeding_edge/test/cctest/test-api.cc > > > This is a semiautomated message from "gvn mail". See > <http://code.google.com/p/gvn/> to learn more. > > Index: include/v8.h > =================================================================== > --- include/v8.h (^/branches/bleeding_edge/include/[EMAIL PROTECTED]) > +++ include/v8.h (^/changes/[EMAIL > PROTECTED]/reenable-most-message-capturing/bleeding_edge/include/[EMAIL > PROTECTED]) > @@ -1993,11 +1993,19 @@ class EXPORT TryCatch { > */ > void SetVerbose(bool value); > > + /** > + * Set whether or not this TryCatch should capture a Message object > + * which holds source information about where the exception > + * occurred. True by default. > + */ > + void SetCaptureMessage(bool value); > + > public: > TryCatch* next_; > void* exception_; > void* message_; > bool is_verbose_; > + bool capture_message_; > }; > > > Index: src/api.cc > =================================================================== > --- src/api.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/api.cc (^/changes/[EMAIL > PROTECTED]/reenable-most-message-capturing/bleeding_edge/src/[EMAIL > PROTECTED]) > @@ -1082,7 +1082,8 @@ v8::TryCatch::TryCatch() > : next_(i::Top::try_catch_handler()), > exception_(i::Heap::the_hole_value()), > message_(i::Smi::FromInt(0)), > - is_verbose_(false) { > + is_verbose_(false), > + capture_message_(true) { > i::Top::RegisterTryCatchHandler(this); > } > > @@ -1129,6 +1130,11 @@ void v8::TryCatch::SetVerbose(bool value) { > } > > > +void v8::TryCatch::SetCaptureMessage(bool value) { > + capture_message_ = value; > +} > + > + > // --- M e s s a g e --- > > > Index: src/execution.cc > =================================================================== > --- src/execution.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/execution.cc (^/changes/[EMAIL > PROTECTED]/reenable-most-message-capturing/bleeding_edge/src/[EMAIL > PROTECTED]) > @@ -130,9 +130,12 @@ Handle<Object> Execution::TryCall(Handle<JSFunctio > Object*** args, > bool* caught_exception) { > // Enter a try-block while executing the JavaScript code. To avoid > - // duplicate error printing it must be non-verbose. > + // duplicate error printing it must be non-verbose. Also, to avoid > + // creating message objects during stack overflow we shouldn't > + // capture messages. > v8::TryCatch catcher; > catcher.SetVerbose(false); > + catcher.SetCaptureMessage(false); > > Handle<Object> result = Invoke(false, func, receiver, argc, args, > caught_exception); > Index: src/top.cc > =================================================================== > --- src/top.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/top.cc (^/changes/[EMAIL > PROTECTED]/reenable-most-message-capturing/bleeding_edge/src/[EMAIL > PROTECTED]) > @@ -770,7 +770,9 @@ void Top::DoThrow(Object* exception, > > Handle<Object> message_obj; > MessageLocation potential_computed_location; > - if (report_exception) { > + bool try_catch_needs_message = > + is_caught_externally && > thread_local_.try_catch_handler_->capture_message_; > + if (report_exception || try_catch_needs_message) { > if (location == NULL) { > // If no location was specified we use a computed one instead > ComputeLocation(&potential_computed_location); > Index: test/cctest/test-api.cc > =================================================================== > --- test/cctest/test-api.cc (^/branches/bleeding_edge/test/cctest/[EMAIL > PROTECTED]) > +++ test/cctest/test-api.cc (^/changes/[EMAIL > PROTECTED]/reenable-most-message-capturing/bleeding_edge/test/cctest/[EMAIL > PROTECTED]) > @@ -4845,6 +4845,21 @@ THREADED_TEST(Regress54) { > } > > > +THREADED_TEST(CatchStackOverflow) { > + v8::HandleScope scope; > + LocalContext context; > + v8::TryCatch try_catch; > + v8::Handle<v8::Script> script = v8::Script::Compile(v8::String::New( > + "function f() {" > + " return f();" > + "}" > + "" > + "f();")); > + v8::Handle<v8::Value> result = script->Run(); > + CHECK(result.IsEmpty()); > +} > + > + > THREADED_TEST(TryCatchSourceInfo) { > v8::HandleScope scope; > LocalContext context; > >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
