http://codereview.chromium.org/9310122/diff/2002/src/isolate.cc File src/isolate.cc (left):
http://codereview.chromium.org/9310122/diff/2002/src/isolate.cc#oldcode1088 src/isolate.cc:1088: ASSERT(is_object); // Can't use the handle unless there's a real object. I seems that DoThrow currently can't be called with Failure object only with real Object. If this is true: please change DoThrow signature to reflect this and remove is_object and related checks. If this is false: there might be a bug here (ASSERT was not here for nothing). http://codereview.chromium.org/9310122/diff/2002/src/isolate.cc File src/isolate.cc (right): http://codereview.chromium.org/9310122/diff/2002/src/isolate.cc#newcode545 src/isolate.cc:545: void Isolate::CaptureAndSetCurrentStackTraceFor(Handle<Object> error_object) { Object -> JSObject as we are expect it to be so. http://codereview.chromium.org/9310122/diff/2002/src/isolate.cc#newcode552 src/isolate.cc:552: JSObject::cast(*error_object)->SetHiddenProperty(key, stack_trace); I think this is point of potential allocation failure which requires handling. (it seems we forgot to annotate SetHiddenProperty with MUST_USE_RESULT to capture it, please annotate it). You can use static method JSObject::SetHiddenProperty instead. http://codereview.chromium.org/9310122/diff/2002/src/isolate.cc#newcode1098 src/isolate.cc:1098: Handle<Object> message_obj; Now can be moved into the if where is needed. http://codereview.chromium.org/9310122/diff/2002/src/isolate.cc#newcode1116 src/isolate.cc:1116: String* key = *(factory()->LookupAsciiSymbol("stackTrace")); "stackTrace" duplicated in two places. Consider moving somewhere (e.g. to roots as symbol, see identity_hash_symbol for example). Also consider prefixing it with v8:: http://codereview.chromium.org/9310122/diff/2002/src/isolate.cc#newcode1119 src/isolate.cc:1119: stack_trace_object = Handle<JSArray>(JSArray::cast(stack_property)); consider code like: throw {__proto__: new Error()}; there will be no hidden property stackTrace on the exception itself. http://codereview.chromium.org/9310122/diff/2002/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/9310122/diff/2002/src/runtime.cc#newcode13267 src/runtime.cc:13267: Handle<Object> error_object = args.at<Object>(0); CONVERT_ARG_CHECKED(JSObject, error_object, 0); just to be sure that we are getting JSObject. http://codereview.chromium.org/9310122/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
