Task tracking bug
Created attachment 20468 [details] Here we go
+++ b/JavaScriptCore/VM/ExceptionHelpers.cpp The machine has a bunch of helper functions right now, all in Machine.cpp. I think it would be best to keep all the helpers there for now, rather than splitting out new source files. Especially because Darin will box our ears if we add a file named "*Helpers" or "*Utilities" to the project :). +static NEVER_INLINE bool resolve(ExecState* exec, Instruction* vPC, Register* r, ScopeChain* scopeChain, CodeBlock* codeBlock, JSValue*& exceptionData) I think "excpetionValue" is a clearer name than "exceptionData". It's not an arbitrary bunch of data -- it's a JSValue*. +static NEVER_INLINE bool notObject(ExecState* exec, const Instruction*, CodeBlock*, JSValue* value, JSValue*& exceptionData) How about "isNotObject"? + if (UNLIKELY(exec->hadException())) {\ Missing space at the end. JSObject* base = r[r1].u.jsValue->toObject(exec); - // FIXME: missing exception check + JSValue* subscript = r[r2].u.jsValue; uint32_t i; if (subscript->getUInt32(i)) r[r0].u.jsValue = base->get(exec, i); - else + else { + VM_CHECK_EXCEPTION(); // This is needed as toString may have side effects r[r0].u.jsValue = base->get(exec, Identifier(subscript->toString(exec))); I think I only understand this comment because you explained what was going on to me in person. How about this: JSObject* base = r[r1].u.jsValue->toObject(exec); // may throw ... else { VM_CHECK_EXCEPTION(); // If toObject threw, we must not call toString, which may execute arbitrary code r[r0].u.jsValue = base->get(exec, Identifier(subscript->toString(exec))); + else { + VM_CHECK_EXCEPTION(); // This is needed as toString may have side effects base->put(exec, Identifier(subscript->toString(exec)), r[r2].u.jsValue); - + } Ditto. + else { + VM_CHECK_EXCEPTION(); // This is needed as toString may have side effects r[r0].u.jsValue = jsBoolean(base->deleteProperty(exec, Identifier(subscript->toString(exec)))); Ditto. + // FIXME throw type error ASSERT(callType == CallTypeNone); - ++vPC; + + exit(0); + vPC++; Ummm... yeah.... no :). - // FIXME: We need to throw a TypeError here if v is not an Object. - ASSERT(v->isObject()); - + ASSERT(callType == CallTypeNone || v->isObject()); Seems like overkill to me, but okay. ASSERT(callType == CallTypeNone); + + if (notObject(exec, vPC, codeBlock, v, exceptionValue)) + goto internal_throw; + // throw type error for non-contructor object + ASSERT(!constructor->implementsConstruct()); // if we get here then v must be an Object that is not a constructor I'm a little confused by the above. It seems no matter how we got here, we should throw a "You can't use X with 'new'" exception. No? + exceptionTarget = throwException(exec, exceptionValue, codeBlock, k, scopeChain, registerBase, r, vPC); + if (!exceptionTarget) { + registerFile->shrink(0); + return exceptionValue; + } No shrink-y, please. + JSValue *InvalidObject::toPrimitive(ExecState *exec, JSType) const + { + UNUSED_PARAM(exec); + ASSERT(exec->hadException() && exec->exception() == m_exception); + return m_exception; + } I don't think this is valid. ToPrimitive is required to return a primitive type. Or does it all work out in the end? + JSObject *InvalidObject::toObject(ExecState *exec) const What's with all the "ExecState *"? Don't you know that the WebKit project was started by a bunch of guys who dreamed to move the "*" on every line of KHTML ever written? +namespace KJS { + class InvalidObject : public JSObject { You're going to think I'm crazy, but hear me out: Let's call this class "JSNotAnObject". I know what you're thinking: "How could JSNotAnObject be a subclass of JSObject?" But, the thing is, in JavaScript, something that is not an object can be used as if it were an object, and that's how this object comes about! For example: (1).x // 1 (not an object) used as an object So, here, 1 "is a" object, to some extent. (Until it throws, anyway). Also might be worth a one-line comment at the top of the file explaining why this unholy object exists. r=me, but definitely no shrink-y and no exit-y, please.
Committed r31798