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]/try-catch-src-info to review the following change: [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-09-09 13:16:57 +-100 (Tue, 09 Sep 2008) Description: Added source info to TryCatches. Reorganized exception messaging somewhat and folded stack traces into message. Use of this in the shell will follow in a separate changelist. Affected Paths: M //branches/bleeding_edge/include/v8.h M //branches/bleeding_edge/src/api.cc M //branches/bleeding_edge/src/debug.cc M //branches/bleeding_edge/src/messages.cc M //branches/bleeding_edge/src/messages.h M //branches/bleeding_edge/src/messages.js M //branches/bleeding_edge/src/top.cc M //branches/bleeding_edge/src/top.h 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]/try-catch-src-info/bleeding_edge/include/[EMAIL PROTECTED]) @@ -553,7 +553,7 @@ class EXPORT Script { class EXPORT Message { public: Local<String> Get(); - Local<Value> GetSourceLine(); + Local<String> GetSourceLine(); // TODO(1241256): Rewrite (or remove) this method. We don't want to // deal with ownership of the returned string and we want to use @@ -566,8 +566,41 @@ class EXPORT Message { // bindings. Handle<Value> GetSourceData(); + /** + * Returns the number, 1-based, of the line where the error occurred. + */ int GetLineNumber(); + /** + * Returns the index within the script of the first character where + * the error occurred. + */ + int GetStartPosition(); + + /** + * Returns the index within the script of the last character where + * the error occurred. + */ + int GetEndPosition(); + + /** + * Returns the index within the line of the first character where + * the error occurred. + */ + int GetStartColumn(); + + /** + * Returns the index within the line of the last character where + * the error occurred. + */ + int GetEndColumn(); + + /** + * Returns a string stack trace if trace_exceptions is enabled and + * one is available. + */ + Local<String> GetStackTrace(); + // TODO(1245381): Print to a string instead of on a FILE. static void PrintCurrentStackTrace(FILE* out); }; @@ -1912,6 +1945,15 @@ class EXPORT TryCatch { Local<Value> Exception(); /** + * Returns the message associated with this exception. If there is + * no message associated an empty handle is returned. + * + * The returned handle is valid until this TryCatch block has been + * destroyed. + */ + Local<v8::Message> Message(); + + /** * Clears any exceptions that may have been caught by this try/catch block. * After this method has been called, HasCaught() will return false. * @@ -1935,6 +1977,7 @@ class EXPORT TryCatch { public: TryCatch* next_; void* exception_; + void* message_; bool is_verbose_; }; Index: src/api.cc =================================================================== --- src/api.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/api.cc (^/changes/[EMAIL PROTECTED]/try-catch-src-info/bleeding_edge/src/[EMAIL PROTECTED]) @@ -1081,6 +1081,7 @@ Local<Value> Script::Run() { v8::TryCatch::TryCatch() : next_(i::Top::try_catch_handler()), exception_(i::Heap::the_hole_value()), + message_(i::Smi::FromInt(0)), is_verbose_(false) { i::Top::RegisterTryCatchHandler(this); } @@ -1107,8 +1108,19 @@ v8::Local<Value> v8::TryCatch::Exception() { } +v8::Local<v8::Message> v8::TryCatch::Message() { + if (HasCaught() && message_ != i::Smi::FromInt(0)) { + i::Object* message = reinterpret_cast<i::Object*>(message_); + return v8::Utils::MessageToLocal(i::Handle<i::Object>(message)); + } else { + return v8::Local<v8::Message>(); + } +} + + void v8::TryCatch::Reset() { exception_ = i::Heap::the_hole_value(); + message_ = i::Smi::FromInt(0); } @@ -1196,15 +1208,80 @@ int Message::GetLineNumber() { } -Local<Value> Message::GetSourceLine() { - ON_BAILOUT("v8::Message::GetSourceLine()", return Local<Value>()); +int Message::GetStartPosition() { + if (IsDeadCheck("v8::Message::GetStartPosition()")) return 0; HandleScope scope; + + i::Handle<i::JSObject> data_obj = Utils::OpenHandle(this); + return static_cast<int>(GetProperty(data_obj, "startPos")->Number()); +} + + +int Message::GetEndPosition() { + if (IsDeadCheck("v8::Message::GetEndPosition()")) return 0; + HandleScope scope; + i::Handle<i::JSObject> data_obj = Utils::OpenHandle(this); + return static_cast<int>(GetProperty(data_obj, "endPos")->Number()); +} + + +int Message::GetStartColumn() { + if (IsDeadCheck("v8::Message::GetStartColumn()")) return 0; + HandleScope scope; + i::Handle<i::JSObject> data_obj = Utils::OpenHandle(this); EXCEPTION_PREAMBLE(); + i::Handle<i::Object> start_col_obj = CallV8HeapFunction( + "GetPositionInLine", + data_obj, + &has_pending_exception); + EXCEPTION_BAILOUT_CHECK(0); + return static_cast<int>(start_col_obj->Number()); +} + + +int Message::GetEndColumn() { + if (IsDeadCheck("v8::Message::GetEndColumn()")) return 0; + HandleScope scope; + i::Handle<i::JSObject> data_obj = Utils::OpenHandle(this); + EXCEPTION_PREAMBLE(); + i::Handle<i::Object> start_col_obj = CallV8HeapFunction( + "GetPositionInLine", + data_obj, + &has_pending_exception); + EXCEPTION_BAILOUT_CHECK(0); + int start = static_cast<int>(GetProperty(data_obj, "startPos")->Number()); + int end = static_cast<int>(GetProperty(data_obj, "endPos")->Number()); + return static_cast<int>(start_col_obj->Number()) + (end - start); +} + + +v8::Local<v8::String> Message::GetStackTrace() { + if (IsDeadCheck("v8::Message::GetStackTrace()")) + return v8::Local<v8::String>(); + HandleScope scope; + i::Handle<i::JSObject> data_obj = Utils::OpenHandle(this); + i::Handle<i::Object> trace = GetProperty(data_obj, "stackTrace"); + if (trace->IsString()) { + return scope.Close(Utils::ToLocal(i::Handle<i::String>::cast(trace))); + } else { + return Local<String>(); + } +} + + +Local<String> Message::GetSourceLine() { + ON_BAILOUT("v8::Message::GetSourceLine()", return Local<String>()); + HandleScope scope; + EXCEPTION_PREAMBLE(); i::Handle<i::Object> result = CallV8HeapFunction("GetSourceLine", Utils::OpenHandle(this), &has_pending_exception); - EXCEPTION_BAILOUT_CHECK(Local<v8::Value>()); - return scope.Close(Utils::ToLocal(result)); + EXCEPTION_BAILOUT_CHECK(Local<v8::String>()); + if (result->IsString()) { + return scope.Close(Utils::ToLocal(i::Handle<i::String>::cast(result))); + } else { + return Local<String>(); + } } Index: src/debug.cc =================================================================== --- src/debug.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/debug.cc (^/changes/[EMAIL PROTECTED]/try-catch-src-info/bleeding_edge/src/[EMAIL PROTECTED]) @@ -537,8 +537,10 @@ bool Debug::CompileDebuggerScript(int index) { // Check for caught exceptions. if (caught_exception) { - MessageHandler::ReportMessage("error_loading_debugger", NULL, - HandleVector<Object>(&result, 1)); + Handle<Object> message = MessageHandler::MakeMessageObject( + "error_loading_debugger", NULL, HandleVector<Object>(&result, 1), + Handle<String>()); + MessageHandler::ReportMessage(NULL, message); return false; } Index: src/messages.cc =================================================================== --- src/messages.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/messages.cc (^/changes/[EMAIL PROTECTED]/try-catch-src-info/bleeding_edge/src/[EMAIL PROTECTED]) @@ -60,8 +60,11 @@ void MessageHandler::ReportMessage(const char* msg } -void MessageHandler::ReportMessage(const char* type, MessageLocation* loc, - Vector< Handle<Object> > args) { +Handle<Object> MessageHandler::MakeMessageObject( + const char* type, + MessageLocation* loc, + Vector< Handle<Object> > args, + Handle<String> stack_trace) { // Build error message object HandleScope scope; Handle<Object> type_str = Factory::LookupAsciiSymbol(type); @@ -82,12 +85,16 @@ void MessageHandler::ReportMessage(const char* msg } Handle<Object> start_handle(Smi::FromInt(start)); Handle<Object> end_handle(Smi::FromInt(end)); - const int argc = 5; + Handle<Object> stack_trace_val = stack_trace.is_null() + ? Factory::undefined_value() + : Handle<Object>::cast(stack_trace); + const int argc = 6; Object** argv[argc] = { type_str.location(), array.location(), start_handle.location(), end_handle.location(), - script.location() }; + script.location(), + stack_trace_val.location() }; bool caught_exception = false; Handle<Object> message = @@ -97,8 +104,13 @@ void MessageHandler::ReportMessage(const char* msg // skip doing the callback. This usually only happens in case of // stack overflow exceptions being thrown by the parser when the // stack is almost full. - if (caught_exception) return; + if (caught_exception) return Handle<Object>(); + return message.EscapeFrom(&scope); +} + +void MessageHandler::ReportMessage(MessageLocation* loc, + Handle<Object> message) { v8::Local<v8::Message> api_message_obj = v8::Utils::MessageToLocal(message); v8::NeanderArray global_listeners(Factory::message_listeners()); Index: src/messages.h =================================================================== --- src/messages.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/messages.h (^/changes/[EMAIL PROTECTED]/try-catch-src-info/bleeding_edge/src/[EMAIL PROTECTED]) @@ -70,6 +70,7 @@ class MessageLocation { : script_(script), start_pos_(start_pos), end_pos_(end_pos) { } + MessageLocation() : start_pos_(-1), end_pos_(-1) { } Handle<Script> script() const { return script_; } int start_pos() const { return start_pos_; } @@ -89,10 +90,14 @@ class MessageHandler { // Report a message (w/o JS heap allocation). static void ReportMessage(const char* msg); + // Returns a message object for the API to use. + static Handle<Object> MakeMessageObject(const char* type, + MessageLocation* loc, + Vector< Handle<Object> > args, + Handle<String> stack_trace); + // Report a formatted message (needs JS allocation). - static void ReportMessage(const char* type, - MessageLocation* loc, - Vector< Handle<Object> > args); + static void ReportMessage(MessageLocation* loc, Handle<Object> message); static void DefaultMessageReport(const MessageLocation* loc, Handle<Object> message_obj); Index: src/messages.js =================================================================== --- src/messages.js (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/messages.js (^/changes/[EMAIL PROTECTED]/try-catch-src-info/bleeding_edge/src/[EMAIL PROTECTED]) @@ -555,17 +555,18 @@ function GetPositionInLine(message) { }; -function ErrorMessage(type, args, startPos, endPos, script) { +function ErrorMessage(type, args, startPos, endPos, script, stackTrace) { this.startPos = startPos; this.endPos = endPos; this.type = type; this.args = args; this.script = script; + this.stackTrace = stackTrace; }; -function MakeMessage(type, args, startPos, endPos, script) { - return new ErrorMessage(type, args, startPos, endPos, script); +function MakeMessage(type, args, startPos, endPos, script, stackTrace) { + return new ErrorMessage(type, args, startPos, endPos, script, stackTrace); }; Index: src/top.cc =================================================================== --- src/top.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/top.cc (^/changes/[EMAIL PROTECTED]/try-catch-src-info/bleeding_edge/src/[EMAIL PROTECTED]) @@ -79,6 +79,7 @@ void Top::Iterate(ObjectVisitor* v, ThreadLocalTop block != NULL; block = block->next_) { VISIT(reinterpret_cast<Object*&>(block->exception_)); + VISIT(reinterpret_cast<Object*&>(block->message_)); } // Iterate over pointers on native execution stack. @@ -671,39 +672,34 @@ void Top::PrintCurrentStackTrace(FILE* out) { } +void Top::ComputeLocation(MessageLocation* target) { + *target = MessageLocation(empty_script(), -1, -1); + StackTraceFrameIterator it; + if (!it.done()) { + JavaScriptFrame* frame = it.frame(); + JSFunction* fun = JSFunction::cast(frame->function()); + Object* script = fun->shared()->script(); + if (script->IsScript() && + !(Script::cast(script)->source()->IsUndefined())) { + int pos = frame->FindCode()->SourcePosition(frame->pc()); + // Compute the location from the function and the reloc info. + Handle<Script> casted_script(Script::cast(script)); + *target = MessageLocation(casted_script, pos, pos + 1); + } + } +} + + void Top::ReportUncaughtException(Handle<Object> exception, MessageLocation* location, Handle<String> stack_trace) { - MessageLocation computed_location(empty_script(), -1, -1); - if (location == NULL) { - location = &computed_location; + Handle<Object> message = MessageHandler::MakeMessageObject( + "uncaught_exception", location, HandleVector<Object>(&exception, 1), + stack_trace); - StackTraceFrameIterator it; - if (!it.done()) { - JavaScriptFrame* frame = it.frame(); - JSFunction* fun = JSFunction::cast(frame->function()); - Object* script = fun->shared()->script(); - if (script->IsScript() && - !(Script::cast(script)->source()->IsUndefined())) { - int pos = frame->FindCode()->SourcePosition(frame->pc()); - // Compute the location from the function and the reloc info. - Handle<Script> casted_script(Script::cast(script)); - computed_location = MessageLocation(casted_script, pos, pos + 1); - } - } - } // Report the uncaught exception. - MessageHandler::ReportMessage("uncaught_exception", - location, - HandleVector<Object>(&exception, 1)); - - // Optionally, report the stack trace separately. - if (!stack_trace.is_null()) { - MessageHandler::ReportMessage("stack_trace", - location, - HandleVector<String>(&stack_trace, 1)); - } + MessageHandler::ReportMessage(location, message); } @@ -771,12 +767,30 @@ void Top::DoThrow(Object* exception, ShouldReportException(&is_caught_externally); if (is_rethrow) report_exception = false; + Handle<Object> message_obj; + MessageLocation potential_computed_location; + if (is_caught_externally || report_exception) { + if (location == NULL) { + // If no location was specified we use a computed one instead + ComputeLocation(&potential_computed_location); + location = &potential_computed_location; + } + Handle<String> stack_trace; + if (FLAG_trace_exception) stack_trace = StackTrace(); + message_obj = MessageHandler::MakeMessageObject("uncaught_exception", + location, HandleVector<Object>(&exception_handle, 1), stack_trace); + } + // 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. if (is_caught_externally) { thread_local_.try_catch_handler_->exception_ = reinterpret_cast<void*>(*exception_handle); + if (!message_obj.is_null()) { + thread_local_.try_catch_handler_->message_ = + reinterpret_cast<void*>(*message_obj); + } } // Notify debugger of exception. @@ -786,9 +800,7 @@ void Top::DoThrow(Object* exception, if (message != NULL) { MessageHandler::ReportMessage(message); } else { - Handle<String> stack_trace; - if (FLAG_trace_exception) stack_trace = StackTrace(); - ReportUncaughtException(exception_handle, location, stack_trace); + MessageHandler::ReportMessage(location, message_obj); } } thread_local_.external_caught_exception_ = is_caught_externally; Index: src/top.h =================================================================== --- src/top.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/top.h (^/changes/[EMAIL PROTECTED]/try-catch-src-info/bleeding_edge/src/[EMAIL PROTECTED]) @@ -226,6 +226,10 @@ class Top { MessageLocation* location, Handle<String> stack_trace); + // Attempts to compute the current source location, storing the + // result in the target out parameter. + static void ComputeLocation(MessageLocation* target); + // Override command line flag. static void TraceException(bool flag); 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]/try-catch-src-info/bleeding_edge/test/cctest/[EMAIL PROTECTED]) @@ -4843,3 +4843,40 @@ THREADED_TEST(Regress54) { v8::Handle<v8::Object> result = templ->NewInstance(); CHECK_EQ(1, result->InternalFieldCount()); } + + +THREADED_TEST(TryCatchSourceInfo) { + v8::HandleScope scope; + LocalContext context; + v8::Handle<v8::String> source = v8::String::New( + "function Foo() {\n" + " return Bar();\n" + "}\n" + "\n" + "function Bar() {\n" + " return Baz();\n" + "}\n" + "\n" + "function Baz() {\n" + " throw 'nirk';\n" + "}\n" + "\n" + "Foo();\n"); + v8::Handle<v8::Script> script = v8::Script::Compile(source, + v8::String::New("test.js")); + v8::TryCatch try_catch; + v8::Handle<v8::Value> result = script->Run(); + CHECK(result.IsEmpty()); + CHECK(try_catch.HasCaught()); + v8::Handle<v8::Message> message = try_catch.Message(); + CHECK(!message.IsEmpty()); + CHECK_EQ(10, message->GetLineNumber()); + CHECK_EQ(91, message->GetStartPosition()); + CHECK_EQ(92, message->GetEndPosition()); + CHECK_EQ(2, message->GetStartColumn()); + CHECK_EQ(3, message->GetEndColumn()); + v8::String::AsciiValue line(message->GetSourceLine()); + CHECK_EQ(" throw 'nirk';", *line); + v8::String::AsciiValue name(message->GetScriptResourceName()); + CHECK_EQ("test.js", *name); +} --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
