Reviewed by Darin.
authorggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Jul 2006 21:17:26 +0000 (21:17 +0000)
committerggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Jul 2006 21:17:26 +0000 (21:17 +0000)
        Improved type safety by implementing opaque JSValue/JSObject typing through
        abuse of 'const', not void*. Also fixed an alarming number of bugs
        exposed by this new type safety.

        I made one design change in JavaScriptCore, which is that the JSObject
        constructor should take a JSValue* as its prototype argument, not a JSObject*,
        since we allow the prototype to be any JSValue*, including jsNull(), for
        example.

        * API/APICast.h:
        (toJS):
        * API/JSBase.h:
        * API/JSCallbackConstructor.cpp:
        (KJS::JSCallbackConstructor::construct):
        * API/JSCallbackFunction.cpp:
        (KJS::JSCallbackFunction::callAsFunction):
        * API/JSCallbackObject.cpp:
        (KJS::JSCallbackObject::JSCallbackObject):
        (KJS::JSCallbackObject::getOwnPropertySlot):
        (KJS::JSCallbackObject::put):
        (KJS::JSCallbackObject::construct):
        (KJS::JSCallbackObject::callAsFunction):
        (KJS::JSCallbackObject::staticFunctionGetter):
        * API/JSCallbackObject.h:
        * API/JSContextRef.cpp:
        (JSEvaluate):
        * API/JSNode.c:
        (JSNodePrototype_appendChild):
        (JSNodePrototype_removeChild):
        (JSNodePrototype_replaceChild):
        * API/JSObjectRef.cpp:
        (JSObjectMake):
        (JSFunctionMakeWithBody):
        (JSObjectGetProperty):
        (JSObjectCallAsFunction):
        (JSObjectCallAsConstructor):
        * API/JSObjectRef.h:
        * API/testapi.c:
        (main):
        * ChangeLog:
        * kjs/object.h:
        (KJS::JSObject::JSObject):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@15310 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
JavaScriptCore/API/APICast.h
JavaScriptCore/API/JSBase.h
JavaScriptCore/API/JSCallbackConstructor.cpp
JavaScriptCore/API/JSCallbackFunction.cpp
JavaScriptCore/API/JSCallbackObject.cpp
JavaScriptCore/API/JSCallbackObject.h
JavaScriptCore/API/JSContextRef.cpp
JavaScriptCore/API/JSNode.c
JavaScriptCore/API/JSObjectRef.cpp
JavaScriptCore/API/JSObjectRef.h
JavaScriptCore/API/testapi.c
JavaScriptCore/ChangeLog
JavaScriptCore/kjs/object.h

index 40e2672e94bef12a4810455f2a7bc709bb773d29..1097ea9d91d354d4ee5aff3eaa20531efe42dfef 100644 (file)
@@ -46,7 +46,7 @@ inline KJS::ExecState* toJS(JSContextRef c)
 
 inline KJS::JSValue* toJS(JSValueRef v)
 {
-    return reinterpret_cast<KJS::JSValue*>(v);
+    return reinterpret_cast<KJS::JSValue*>(const_cast<__JSValue*>(v));
 }
 
 inline KJS::UString::Rep* toJS(JSInternalStringRef b)
index ee6773d732880eee89fdde7b7efa53f87ad46fea..3c3926bd4fa0bac7ce7baa27a753046a97955071 100644 (file)
@@ -44,9 +44,9 @@ typedef struct __JSPropertyEnumerator* JSPropertyEnumeratorRef;
 /* JavaScript data types */
 
 /*! @typedef JSValueRef A JavaScript value. The base type for all JavaScript values, and polymorphic functions on them. */
-typedef void* JSValueRef;
+typedef const struct __JSValue* JSValueRef;
 
 /*! @typedef JSObjectRef A JavaScript object. A JSObject is a JSValue. */
-typedef struct __JSObject* JSObjectRef;
+typedef struct __JSValue* JSObjectRef;
 
 #endif // JSBase_h
index 910ff900e91d4f4f5fc4398bd33b20145eeb8f23..f24b8d9efb14668a9f7cf8c380b0341d3aa4d15c 100644 (file)
@@ -50,7 +50,7 @@ JSObject* JSCallbackConstructor::construct(ExecState* exec, const List &args)
     size_t argc = args.size();
     JSValueRef argv[argc];
     for (size_t i = 0; i < argc; i++)
-        argv[i] = args[i];
+        argv[i] = toRef(args[i]);
     return toJS(m_callback(execRef, thisRef, argc, argv));
 }
 
index ec0f1d1b1166dde965577759d96c5f6be8b3fc24..b22f4d0972e950da6007a1c760d1529d2a982b42 100644 (file)
@@ -53,7 +53,7 @@ JSValue* JSCallbackFunction::callAsFunction(ExecState* exec, JSObject* thisObj,
     size_t argc = args.size();
     JSValueRef argv[argc];
     for (size_t i = 0; i < argc; i++)
-        argv[i] = args[i];
+        argv[i] = toRef(args[i]);
     return toJS(m_callback(execRef, thisRef, thisObjRef, argc, argv));
 }
 
index 2213bcf4f0fabb192d80786dce854dc80a3c2d1b..bf92697ab1bcadea00bdefdfbeda5364de77075a 100644 (file)
@@ -43,7 +43,7 @@ JSCallbackObject::JSCallbackObject(JSContextRef context, JSClassRef jsClass)
     init(context, jsClass);
 }
 
-JSCallbackObject::JSCallbackObject(JSContextRef context, JSClassRef jsClass, JSObject* prototype)
+JSCallbackObject::JSCallbackObject(JSContextRef context, JSClassRef jsClass, JSValue* prototype)
     : JSObject(prototype)
 {
     init(context, jsClass);
@@ -97,7 +97,7 @@ bool JSCallbackObject::getOwnPropertySlot(ExecState* exec, const Identifier& pro
                 // cache the value so we don't have to compute it again
                 // FIXME: This violates the PropertySlot design a little bit.
                 // We should either use this optimization everywhere, or nowhere.
-                slot.setCustom(reinterpret_cast<JSObject*>(returnValue), cachedValueGetter);
+                slot.setCustom(reinterpret_cast<JSObject*>(toJS(returnValue)), cachedValueGetter);
                 return true;
             }
         }
@@ -132,10 +132,11 @@ void JSCallbackObject::put(ExecState* exec, const Identifier& propertyName, JSVa
     JSContextRef context = toRef(exec);
     JSObjectRef thisRef = toRef(this);
     JSInternalStringRef propertyNameRef = toRef(propertyName.ustring().rep());
+    JSValueRef valueRef = toRef(value);
 
     for (JSClassRef jsClass = m_class; jsClass; jsClass = jsClass->parent) {
         if (JSSetPropertyCallback setPropertyCallback = jsClass->callbacks.setProperty) {
-            if (setPropertyCallback(context, thisRef, propertyNameRef, value))
+            if (setPropertyCallback(context, thisRef, propertyNameRef, valueRef))
                 return;
         }
     
@@ -144,7 +145,7 @@ void JSCallbackObject::put(ExecState* exec, const Identifier& propertyName, JSVa
                 if (entry->attributes & kJSPropertyAttributeReadOnly)
                     return;
                 if (JSSetPropertyCallback setPropertyCallback = entry->setProperty) {
-                    if (setPropertyCallback(context, thisRef, propertyNameRef, value))
+                    if (setPropertyCallback(context, thisRef, propertyNameRef, valueRef))
                         return;
                 }
             }
@@ -222,7 +223,7 @@ JSObject* JSCallbackObject::construct(ExecState* exec, const List& args)
             size_t argc = args.size();
             JSValueRef argv[argc];
             for (size_t i = 0; i < argc; i++)
-                argv[i] = args[i];
+                argv[i] = toRef(args[i]);
             return toJS(callAsConstructorCallback(execRef, thisRef, argc, argv));
         }
     }
@@ -251,7 +252,7 @@ JSValue* JSCallbackObject::callAsFunction(ExecState* exec, JSObject* thisObj, co
             size_t argc = args.size();
             JSValueRef argv[argc];
             for (size_t i = 0; i < argc; i++)
-                argv[i] = args[i];
+                argv[i] = toRef(args[i]);
             return toJS(callAsFunctionCallback(execRef, thisRef, thisObjRef, argc, argv));
         }
     }
@@ -392,7 +393,7 @@ JSValue* JSCallbackObject::staticFunctionGetter(ExecState* exec, JSObject*, cons
     JSCallbackObject* thisObj = static_cast<JSCallbackObject*>(slot.slotBase());
 
     if (JSValue* cachedOrOverrideValue = thisObj->getDirect(propertyName))
-        return toJS(cachedOrOverrideValue);
+        return cachedOrOverrideValue;
 
     for (JSClassRef jsClass = thisObj->m_class; jsClass; jsClass = jsClass->parent) {
         if (__JSClass::StaticFunctionsTable* staticFunctions = jsClass->staticFunctions) {
index d8499eec5abbc84a34aba5d19965a0c93d842975..77a5b1b3a7160a4bd3dbf8685fdabe4d9ac98ab4 100644 (file)
@@ -37,7 +37,7 @@ class JSCallbackObject : public JSObject
 {
 public:
     JSCallbackObject(JSContextRef, JSClassRef);
-    JSCallbackObject(JSContextRef, JSClassRef, JSObject* prototype);
+    JSCallbackObject(JSContextRef, JSClassRef, JSValue* prototype);
     virtual ~JSCallbackObject();
         
     virtual UString className() const;
index 58d01c73b2966235b61a653825e387f99f972189..9c4dcd2fb91d21e193bb3f8595192bc0acaafed7 100644 (file)
@@ -74,7 +74,7 @@ JSValueRef JSEvaluate(JSContextRef context, JSInternalStringRef script, JSObject
 
     if (completion.complType() == Throw) {
         if (exception)
-            *exception = completion.value();
+            *exception = toRef(completion.value());
         return 0;
     }
     
index e78317b9f2c86ed53972587ab59bfb702c1dffab..3f80cef561c0e249b5a7514aa620e69571a81b24 100644 (file)
@@ -48,7 +48,7 @@ static JSValueRef JSNodePrototype_appendChild(JSContextRef context, JSObjectRef
         JSInternalStringRelease(messageBuf);
     } else {
         Node* node = JSObjectGetPrivate(thisObject);
-        Node* child = JSObjectGetPrivate(argv[0]);
+        Node* child = JSObjectGetPrivate(JSValueToObject(context, argv[0]));
 
         Node_appendChild(node, child);
     }
@@ -66,7 +66,7 @@ static JSValueRef JSNodePrototype_removeChild(JSContextRef context, JSObjectRef
         if (JSValueIsObjectOfClass(thisObject, JSNode_class(context))) {
             if (JSValueIsObjectOfClass(argv[0], JSNode_class(context))) {
                 Node* node = JSObjectGetPrivate(thisObject);
-                Node* child = JSObjectGetPrivate(argv[0]);
+                Node* child = JSObjectGetPrivate(JSValueToObject(context, argv[0]));
                 
                 Node_removeChild(node, child);
             }
@@ -86,8 +86,8 @@ static JSValueRef JSNodePrototype_replaceChild(JSContextRef context, JSObjectRef
             if (JSValueIsObjectOfClass(argv[0], JSNode_class(context))) {
                 if (JSValueIsObjectOfClass(argv[1], JSNode_class(context))) {
                     Node* node = JSObjectGetPrivate(thisObject);
-                    Node* newChild = JSObjectGetPrivate(argv[0]);
-                    Node* oldChild = JSObjectGetPrivate(argv[1]);
+                    Node* newChild = JSObjectGetPrivate(JSValueToObject(context, argv[0]));
+                    Node* oldChild = JSObjectGetPrivate(JSValueToObject(context, argv[1]));
                     
                     Node_replaceChild(node, newChild, oldChild);
                 }
index 967d42a90b807a3523621724417886c9028c5b81..505cf19f12ad3ef2cf2ba96fbd6017c5555c6ecb 100644 (file)
 
 using namespace KJS;
 
-JSObjectRef JSObjectMake(JSContextRef context, JSClassRef jsClass, JSObjectRef prototype)
+JSObjectRef JSObjectMake(JSContextRef context, JSClassRef jsClass, JSValueRef prototype)
 {
     JSLock lock;
 
     ExecState* exec = toJS(context);
-    JSObject* jsPrototype = toJS(prototype);
+    JSValue* jsPrototype = toJS(prototype);
 
     if (!prototype)
         jsPrototype = exec->lexicalInterpreter()->builtinObjectPrototype();
@@ -87,7 +87,7 @@ JSObjectRef JSFunctionMakeWithBody(JSContextRef context, JSInternalStringRef bod
     RefPtr<FunctionBodyNode> bodyNode = Parser::parse(jsSourceURL, startingLineNumber, bodyRep->data(), bodyRep->size(), &sid, &errLine, &errMsg);
     if (!bodyNode) {
         if (exception)
-            *exception = Error::create(exec, SyntaxError, errMsg, errLine, sid, jsSourceURL);
+            *exception = toRef(Error::create(exec, SyntaxError, errMsg, errLine, sid, jsSourceURL));
         return 0;
     }
 
@@ -137,7 +137,7 @@ JSValueRef JSObjectGetProperty(JSContextRef context, JSObjectRef object, JSInter
     JSValue* jsValue = jsObject->get(exec, Identifier(nameRep));
     if (jsValue->isUndefined())
         return 0;
-    return jsValue;
+    return toRef(jsValue);
 }
 
 bool JSObjectSetProperty(JSContextRef context, JSObjectRef object, JSInternalStringRef propertyName, JSValueRef value, JSPropertyAttributes attributes)
@@ -227,7 +227,7 @@ JSValueRef JSObjectCallAsFunction(JSContextRef context, JSObjectRef object, JSOb
     JSValueRef result = toRef(jsObject->call(exec, jsThisObject, argList)); // returns NULL if object->implementsCall() is false
     if (exec->hadException()) {
         if (exception)
-            *exception = exec->exception();
+            *exception = toRef(exec->exception());
         exec->clearException();
         result = 0;
     }
@@ -253,7 +253,7 @@ JSObjectRef JSObjectCallAsConstructor(JSContextRef context, JSObjectRef object,
     JSObjectRef result = toRef(jsObject->construct(exec, argList)); // returns NULL if object->implementsCall() is false
     if (exec->hadException()) {
         if (exception)
-            *exception = exec->exception();
+            *exception = toRef(exec->exception());
         exec->clearException();
         result = 0;
     }
index 67a0b01db3073f795301644827be4e66c7095b6c..ced97b1ddc3de3c8df5690c3c82e327c8a90a6a6 100644 (file)
@@ -316,7 +316,7 @@ void JSClassRelease(JSClassRef jsClass);
 @param prototype The prototype to assign to the object. Pass NULL to use the default object prototype.
 @result A JSObject with the given class and prototype.
 */
-JSObjectRef JSObjectMake(JSContextRef context, JSClassRef jsClass, JSObjectRef prototype);
+JSObjectRef JSObjectMake(JSContextRef context, JSClassRef jsClass, JSValueRef prototype);
 
 /*!
 @function
index 879a4ae217f25c4f7bbfd2a35adfe6c243b67b46..dbd780e4187e3c5d61025e49a7ad6c612178e1e7 100644 (file)
@@ -552,7 +552,7 @@ int main(int argc, char* argv[])
     JSInternalStringRef lineBuf = JSInternalStringCreateUTF8("line");
     assert(!JSFunctionMakeWithBody(context, functionBuf, NULL, 1, &exception));
     assert(JSValueIsObject(exception));
-    v = JSObjectGetProperty(context, exception, lineBuf);
+    v = JSObjectGetProperty(context, JSValueToObject(context, exception), lineBuf);
     assert(v);
     assertEqualsAsNumber(v, 2); // FIXME: Lexer::setCode bumps startingLineNumber by 1 -- we need to change internal callers so that it doesn't have to (saying '0' to mean '1' in the API would be really confusing -- it's really confusing internally, in fact)
     JSInternalStringRelease(functionBuf);
@@ -573,7 +573,7 @@ int main(int argc, char* argv[])
     JSInternalStringRelease(myObjectBuf);
 
     JSInternalStringRef printBuf = JSInternalStringCreateUTF8("print");
-    JSValueRef printFunction = JSFunctionMake(context, print_callAsFunction);
+    JSObjectRef printFunction = JSFunctionMake(context, print_callAsFunction);
     JSObjectSetProperty(context, globalObject, printBuf, printFunction, kJSPropertyAttributeNone); 
     JSInternalStringRelease(printBuf);
     
@@ -581,7 +581,7 @@ int main(int argc, char* argv[])
     assert(JSObjectGetPrivate(printFunction) == (void*)1);
 
     JSInternalStringRef myConstructorBuf = JSInternalStringCreateUTF8("MyConstructor");
-    JSValueRef myConstructor = JSConstructorMake(context, myConstructor_callAsConstructor);
+    JSObjectRef myConstructor = JSConstructorMake(context, myConstructor_callAsConstructor);
     JSObjectSetProperty(context, globalObject, myConstructorBuf, myConstructor, kJSPropertyAttributeNone);
     JSInternalStringRelease(myConstructorBuf);
     
@@ -589,8 +589,8 @@ int main(int argc, char* argv[])
     assert(JSObjectGetPrivate(myConstructor) == (void*)1);
     
     o = JSObjectMake(context, NULL, NULL);
-    JSObjectSetProperty(context, o, jsOneString, JSNumberMake(1), kJSPropertyAttributeNone);
-    JSObjectSetProperty(context, o, jsCFString,  JSNumberMake(1), kJSPropertyAttributeDontEnum);
+    JSObjectSetProperty(context, o, jsOneStringBuf, JSNumberMake(1), kJSPropertyAttributeNone);
+    JSObjectSetProperty(context, o, jsCFStringBuf,  JSNumberMake(1), kJSPropertyAttributeDontEnum);
     JSPropertyEnumeratorRef enumerator = JSObjectCreatePropertyEnumerator(context, o);
     int count = 0;
     while (JSPropertyEnumeratorGetNext(enumerator))
index a200aebe78110723eca468cb83ca3f9c4f3aab63..2f0bd77f87d1181f7702f4865b94fc77c8e151c6 100644 (file)
@@ -1,3 +1,50 @@
+2006-07-10  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Darin.
+
+        Improved type safety by implementing opaque JSValue/JSObject typing through 
+        abuse of 'const', not void*. Also fixed an alarming number of bugs
+        exposed by this new type safety.
+        
+        I made one design change in JavaScriptCore, which is that the JSObject
+        constructor should take a JSValue* as its prototype argument, not a JSObject*,
+        since we allow the prototype to be any JSValue*, including jsNull(), for
+        example.
+        
+        * API/APICast.h:
+        (toJS):
+        * API/JSBase.h:
+        * API/JSCallbackConstructor.cpp:
+        (KJS::JSCallbackConstructor::construct):
+        * API/JSCallbackFunction.cpp:
+        (KJS::JSCallbackFunction::callAsFunction):
+        * API/JSCallbackObject.cpp:
+        (KJS::JSCallbackObject::JSCallbackObject):
+        (KJS::JSCallbackObject::getOwnPropertySlot):
+        (KJS::JSCallbackObject::put):
+        (KJS::JSCallbackObject::construct):
+        (KJS::JSCallbackObject::callAsFunction):
+        (KJS::JSCallbackObject::staticFunctionGetter):
+        * API/JSCallbackObject.h:
+        * API/JSContextRef.cpp:
+        (JSEvaluate):
+        * API/JSNode.c:
+        (JSNodePrototype_appendChild):
+        (JSNodePrototype_removeChild):
+        (JSNodePrototype_replaceChild):
+        * API/JSObjectRef.cpp:
+        (JSObjectMake):
+        (JSFunctionMakeWithBody):
+        (JSObjectGetProperty):
+        (JSObjectCallAsFunction):
+        (JSObjectCallAsConstructor):
+        * API/JSObjectRef.h:
+        * API/testapi.c:
+        (main):
+        * ChangeLog:
+        * kjs/object.h:
+        (KJS::JSObject::JSObject):
+
 2006-07-10  Geoffrey Garen  <ggaren@apple.com>
 
         Approved by Maciej, Darin.
index 4dcf56a583abe5913694bbe339373bc416c9bc38..2c6f2c398332c6f101c74db9a04a8ecd65861d8d 100644 (file)
@@ -105,7 +105,7 @@ namespace KJS {
      *
      * @param proto The prototype
      */
-    JSObject(JSObject *proto, bool destructorIsThreadSafe = true);
+    JSObject(JSValue* proto, bool destructorIsThreadSafe = true);
 
     /**
      * Creates a new JSObject with a prototype of jsNull()
@@ -567,7 +567,7 @@ JSObject *throwError(ExecState *, ErrorType, const UString &message);
 JSObject *throwError(ExecState *, ErrorType, const char *message);
 JSObject *throwError(ExecState *, ErrorType);
 
-inline JSObject::JSObject(JSObject *proto, bool destructorIsThreadSafe)
+inline JSObject::JSObject(JSValue* proto, bool destructorIsThreadSafe)
     : JSCell(destructorIsThreadSafe)
     , _proto(proto)
     , _internalValue(0)