Commits

Ronald Oussoren  committed 8002fa8

This fixes two issues:

1) __slots__ values were not correctly released when the object is dealloced

2) it is now possible to override setValue:forKey: and friends in Python
classes that directly inherit from Objective-C classes. It was already
possible to do so if the parent is implemented in Python.

  • Participants
  • Parent commits 04f6bf3
  • Branches pyobjc-ancient

Comments (0)

Files changed (2)

File Lib/Foundation/test/test_keyvalue.py

     def setKey5_(self, value):
         self.key5 = value * 5
 
+class KeyValueClass1Explicit (NSObject):
+    def init(self):
+        self = super(KeyValueClass1Explicit, self).init()
+        self._values = {}
+        self._values[u'key3'] = 3
+        self._values[u'key4'] = u"4"
+        self._values['_private'] = u"private"
+        return self
+
+    def addMultiple(self):
+        self._values["multiple"] = KeyValueClass1Explicit.alloc().init()
+        self._values["multiple"]._values["level2"] = KeyValueClass1Explicit.alloc().init()
+        self._values["multiple"]._values["level2"]._values["level3"] = KeyValueClass1Explicit.alloc().init()
+        self._values["multiple"]._values["level2"]._values["level3"]._values["keyA"] = u"hello"
+        self._values["multiple"]._values["level2"]._values["level3"]._values["keyB"] = u"world"
+
+    def valueForKey_(self, key):
+        if key == "key1":
+            return 1
+
+        elif key == "key2":
+            return 2
+
+        return self._values[key]
+
+    def storedValueForKey_(self, key):
+        return self.valueForKey_(key)
+
+    def setValue_forKey_(self, value, key):
+        if key == "key4":
+            value = value * 4
+        elif key == "key5":
+            value = value * 5
+
+        self._values[key] = value
+
+    def takeStoredValue_forKey_(self, value, key):
+        self.setValue_forKey_(value, key)
+
+    def takeValue_forKey_(self, value, key):
+        self.setValue_forKey_(value, key)
+
 class KeyValueClass4 (NSObject):
     __slots__ = ('foo', )
 
             finally:
                 o.removeObserver_forKeyPath_(observer, u'key3')
 
+class PyKeyValueCodingExplicit (unittest.TestCase):
+
+    def testValueForKey(self):
+        o = KeyValueClass1Explicit.alloc().init()
+        o.addMultiple()
+
+        self.assertEquals(STUB.keyValue_forObject_key_(0, o, u"key1"), 1)
+
+        self.assertEquals(STUB.keyValue_forObject_key_(0, o, u"key2"), 2)
+        self.assertEquals(STUB.keyValue_forObject_key_(0, o, u"key3"), 3)
+        self.assertEquals(STUB.keyValue_forObject_key_(0, o, u"key4"), "4")
+        self.assertEquals(STUB.keyValue_forObject_key_(0, o, u"multiple"), o._values['multiple'])
+
+        self.assertRaises(KeyError, STUB.keyValue_forObject_key_, 0, o, u"nokey")
+
+    def testStoredValueForKey(self):
+        o = KeyValueClass1Explicit.alloc().init()
+        o.addMultiple()
+
+        self.assertEquals(STUB.keyValue_forObject_key_(2, o, u"key1"), 1)
+        self.assertEquals(STUB.keyValue_forObject_key_(2, o, u"key2"), 2)
+        self.assertEquals(STUB.keyValue_forObject_key_(2, o, u"key3"), 3)
+        self.assertEquals(STUB.keyValue_forObject_key_(2, o, u"key4"), "4")
+        self.assertEquals(STUB.keyValue_forObject_key_(2, o, u"multiple"), o._values['multiple'])
+
+        self.assertRaises(KeyError, STUB.keyValue_forObject_key_, 2, o, u"nokey")
+
+    def testValueForKeyPath(self):
+        o = KeyValueClass1Explicit.alloc().init()
+        o.addMultiple()
+
+        self.assertEquals(STUB.keyValue_forObject_key_(1, o, u"multiple"), o._values['multiple'])
+        self.assertEquals(STUB.keyValue_forObject_key_(1, o, u"multiple.level2"), o._values['multiple']._values['level2'])
+        self.assertEquals(STUB.keyValue_forObject_key_(1, o, u"multiple.level2.level3.keyA"),
+            o._values['multiple']._values['level2']._values['level3']._values['keyA'])
+        self.assertEquals(STUB.keyValue_forObject_key_(1, o, u"multiple.level2.level3.keyB"),
+            o._values['multiple']._values['level2']._values['level3']._values['keyB'])
+
+        self.assertRaises(KeyError, STUB.keyValue_forObject_key_, 1, o, u"multiple.level2.nokey")
+
+    def testValuesForKeys(self):
+        o = KeyValueClass1Explicit.alloc().init()
+
+        self.assertEquals(STUB.keyValue_forObject_key_(3, o, [u"key1", u"key2", u"key3", u"key4"]), { u"key1":1, u"key2":2, u"key3": 3, u"key4": u"4"} )
+
+        self.assertRaises(KeyError, STUB.keyValue_forObject_key_, 3, o, [u"key1", u"key3", u"nosuchkey"])
+
+    def testTakeValueForKey(self):
+        o = KeyValueClass1Explicit.alloc().init()
+
+        self.assertEquals(o._values['key3'], 3)
+        STUB.setKeyValue_forObject_key_value_(0, o, u'key3', u'drie')
+        self.assertEquals(o._values['key3'], u"drie")
+
+        self.assertEquals(o._values['key4'], u"4")
+        STUB.setKeyValue_forObject_key_value_(0, o, u'key4', u'vier')
+        self.assert_(not hasattr(o, u"key4"))
+        self.assertEquals(o._values['key4'], u"viervierviervier")
+
+        o._values['key5'] = 1
+        STUB.setKeyValue_forObject_key_value_(0, o, u'key5', u'V')
+        self.assertEquals(o._values['key5'], u"VVVVV")
+
+        self.assert_(not hasattr(o, u'key9'))
+        self.assert_('key9' not in o._values)
+        STUB.setKeyValue_forObject_key_value_(0, o, u'key9', u'IX')
+        self.assert_(not hasattr(o, u'key9'))
+        self.assert_('key9' in o._values)
+        self.assertEquals(o._values['key9'], u'IX')
+
+    def testTakeStoredValueForKey(self):
+        o = KeyValueClass1Explicit.alloc().init()
+
+        self.assertEquals(o._values['key3'], 3)
+        STUB.setKeyValue_forObject_key_value_(2, o, u'key3', u'drie')
+        self.assertEquals(o._values['key3'], u"drie")
+
+        self.assertEquals(o._values['key4'], u"4")
+        STUB.setKeyValue_forObject_key_value_(2, o, u'key4', u'vier')
+        self.assertEquals(o._values['key4'], u"viervierviervier")
+
+        o.key5 = 1
+        STUB.setKeyValue_forObject_key_value_(2, o, u'key5', u'V')
+        self.assertEquals(o._values['key5'], u"VVVVV")
+
+        self.assert_('key9' not in o._values)
+        STUB.setKeyValue_forObject_key_value_(2, o, u'key9', u'IX')
+        self.assert_('key9' in o._values)
+        self.assertEquals(o._values['key9'], u'IX')
+
+    def testTakeValuesFromDictionary(self):
+        o = KeyValueClass1Explicit.alloc().init()
+
+        self.assertEquals(o._values['key3'], 3)
+        self.assertEquals(o._values['key4'], u"4")
+        o._values['key5'] = 1
+        self.assert_('key9' not in o._values)
+
+        STUB.setKeyValue_forObject_key_value_(3, o, None,
+            {
+                u'key3': u'drie',
+                u'key4': u'vier',
+                u'key5': u'V',
+                u'key9': u'IX',
+            })
+
+        self.assertEquals(o._values['key3'], u"drie")
+        self.assertEquals(o._values['key4'], u"viervierviervier")
+        self.assertEquals(o._values['key5'], u"VVVVV")
+        self.assertEquals(o._values['key9'], u'IX')
+
+    def testTakeValueForKeyPath(self):
+        o = KeyValueClass1Explicit.alloc().init()
+        o.addMultiple()
+
+        self.assertEquals(o._values['multiple']._values['level2']._values['level3']._values['keyA'], u"hello")
+        self.assertEquals(o._values['multiple']._values['level2']._values['level3']._values['keyB'], u"world")
+
+        STUB.setKeyValue_forObject_key_value_(1, o, u"multiple.level2.level3.keyA", u"KeyAValue")
+        self.assertEquals(o._values['multiple']._values['level2']._values['level3']._values['keyA'], u"KeyAValue")
+
+        STUB.setKeyValue_forObject_key_value_(1, o, u"multiple.level2.level3.keyB", 9.999)
+        self.assertEquals(o._values['multiple']._values['level2']._values['level3']._values['keyB'], 9.999)
+
+
 class TestBaseExceptions (unittest.TestCase):
     """
     Check that NSObject implementation of Key-Value coding raises the

File Modules/objc/class-builder.m

 		if (v == NULL) {
 			return -1;
 		}
-		((PyObjCInstanceVariable*)v)->type = PyObjCUtil_Strdup("");
+		((PyObjCInstanceVariable*)v)->type = PyObjCUtil_Strdup(@encode(PyObject*));
 		((PyObjCInstanceVariable*)v)->isSlot = 1;
 		if (PyDict_SetItemString(clsdict, "__dict__", v) < 0) {
 			Py_DECREF(v);
 			Py_DECREF(slots);
 			return -1;
 		}
-		((PyObjCInstanceVariable*)var)->type = PyObjCUtil_Strdup("");
+		((PyObjCInstanceVariable*)var)->type = PyObjCUtil_Strdup(@encode(PyObject*));
 		((PyObjCInstanceVariable*)var)->isSlot = 1;
 	
 		if (PyDict_SetItem(clsdict, slot_value, (PyObject*)var) < 0) {
 	IMP closure;
 	PyObjCMethodSignature* methinfo = NULL;
 
-	method_list = PyObjCRT_AllocMethodList(2);
+	method_list = PyObjCRT_AllocMethodList(9);
 
 	if (method_list == NULL) {
 		PyErr_NoMemory();
 	meth = method_list->method_list + method_list->method_count++;
 	PyObjCRT_InitMethod(meth, @selector(dealloc), "v@:", (IMP)closure); 
 
+	methinfo = PyObjCMethodSignature_FromSignature("@@:@");
+	if (methinfo == NULL) goto error_cleanup; 
+	closure = PyObjCFFI_MakeClosure(methinfo, object_method_valueForKey_,
+		base_class);
+	PyObjCMethodSignature_Free(methinfo); methinfo = NULL;
+	if (closure == NULL) goto error_cleanup;
+	meth = method_list->method_list + method_list->method_count++;
+	PyObjCRT_InitMethod(meth, @selector(valueForKey:), "@@:@", (IMP)closure); 
+	PyObjCRT_InitMethod(meth, @selector(storedValueForKey:), "@@:@", (IMP)closure); 
+
+	methinfo = PyObjCMethodSignature_FromSignature("v@:@@");
+	if (methinfo == NULL) goto error_cleanup; 
+	closure = PyObjCFFI_MakeClosure(methinfo, object_method_setValue_forKey_,
+		base_class);
+	PyObjCMethodSignature_Free(methinfo); methinfo = NULL;
+	if (closure == NULL) goto error_cleanup;
+	meth = method_list->method_list + method_list->method_count++;
+	PyObjCRT_InitMethod(meth, @selector(setValue:forKey:), "v@:@@", (IMP)closure); 
+	PyObjCRT_InitMethod(meth, @selector(takeStoredValue:forKey:), "v@:@@", (IMP)closure); 
+	PyObjCRT_InitMethod(meth, @selector(takeValue:forKey:), "v@:@@", (IMP)closure); 
+
+	if (_KVOHackLevel() == 1) {
+		methinfo = PyObjCMethodSignature_FromSignature("v@:@");
+		if (methinfo == NULL) goto error_cleanup; 
+		closure = PyObjCFFI_MakeClosure(methinfo, object_method_willOrDidChangeValueForKey_,
+			base_class);
+		PyObjCMethodSignature_Free(methinfo); methinfo = NULL;
+		if (closure == NULL) goto error_cleanup;
+		meth = method_list->method_list + method_list->method_count++;
+		PyObjCRT_InitMethod(meth, @selector(willChangeValueForKey:), "v@:@@", (IMP)closure); 
+		PyObjCRT_InitMethod(meth, @selector(didChangeValueForKey:), "v@:@@", (IMP)closure); 
+	}
+
 	root_class = base_class;
 	while (root_class->super_class != NULL) {
 		root_class = root_class->super_class;
 	} else {
 		need_intermediate = 1;
 	}
+	if (PyDict_GetItemString(class_dict, "valueForKey_") == NULL) {
+		PyErr_Clear();
+	} else {
+		need_intermediate = 1;
+	}
+	if (PyDict_GetItemString(class_dict, "storedValueForKey_") == NULL) {
+		PyErr_Clear();
+	} else {
+		need_intermediate = 1;
+	}
+	if (PyDict_GetItemString(class_dict, "setValueForKey_") == NULL) {
+		PyErr_Clear();
+	} else {
+		need_intermediate = 1;
+	}
+	if (PyDict_GetItemString(class_dict, "takeValueForKey_") == NULL) {
+		PyErr_Clear();
+	} else {
+		need_intermediate = 1;
+	}
+	if (PyDict_GetItemString(class_dict, "takeStoredValueForKey_") == NULL) {
+		PyErr_Clear();
+	} else {
+		need_intermediate = 1;
+	}
+	if (_KVOHackLevel() == 1) {
+		if (PyDict_GetItemString(class_dict, "willChangeValueForKey_") == NULL) {
+			PyErr_Clear();
+		} else {
+			need_intermediate = 1;
+		}
+		if (PyDict_GetItemString(class_dict, "didChangeValueForKey_") == NULL) {
+			PyErr_Clear();
+		} else {
+			need_intermediate = 1;
+		}
+	}
 
 	if (!PyObjCClass_HasPythonImplementation(py_superclass) && need_intermediate) {
 		Class intermediate_class;
 				@selector(dealloc), 
 				"v@:", 
 				object_method_dealloc);
+			METH(
+				"storedValueForKey_",
+				@selector(storedValueForKey:),
+				"@@:@",
+				object_method_valueForKey_);
+			METH(
+				"valueForKey_",
+				@selector(valueForKey:),
+				"@@:@",
+				object_method_valueForKey_);
+			METH(
+				"takeStoredValue_forKey_",
+				@selector(takeStoredValue:forKey:),
+				"v@:@@",
+				object_method_setValue_forKey_);
+			METH(
+				"takeValue_forKey_",
+				@selector(takeValue:forKey:),
+				"v@:@@",
+				object_method_setValue_forKey_);
+			METH(
+				"setValue_forKey_",
+				@selector(setValue:forKey:),
+				"v@:@@",
+				object_method_setValue_forKey_);
+			if (_KVOHackLevel() == 1) {
+				METH(
+					"willChangeValueForKey_",
+					@selector(willChangeValueForKey:),
+					"v@:@",
+					object_method_willOrDidChangeValueForKey_);
+				METH(
+					"didChangeValueForKey_",
+					@selector(didChangeValueForKey:),
+					"v@:@",
+					object_method_willOrDidChangeValueForKey_);
+			}
 		}
 		/* FIXME: 
 		 * all these should be in the intermediate class as well,
 			@selector(forwardInvocation:), 
 			"v@:@", 
 			object_method_forwardInvocation);
-		METH(
-			"storedValueForKey_",
-			@selector(storedValueForKey:),
-			"@@:@",
-			object_method_valueForKey_);
-		METH(
-			"valueForKey_",
-			@selector(valueForKey:),
-			"@@:@",
-			object_method_valueForKey_);
-		METH(
-			"takeStoredValue_forKey_",
-			@selector(takeStoredValue:forKey:),
-			"v@:@@",
-			object_method_setValue_forKey_);
-		METH(
-			"takeValue_forKey_",
-			@selector(takeValue:forKey:),
-			"v@:@@",
-			object_method_setValue_forKey_);
-		METH(
-			"setValue_forKey_",
-			@selector(setValue:forKey:),
-			"v@:@@",
-			object_method_setValue_forKey_);
-		if (_KVOHackLevel() == 1) {
-			METH(
-				"willChangeValueForKey_",
-				@selector(willChangeValueForKey:),
-				"v@:@",
-				object_method_willOrDidChangeValueForKey_);
-			METH(
-				"didChangeValueForKey_",
-				@selector(didChangeValueForKey:),
-				"v@:@",
-				object_method_willOrDidChangeValueForKey_);
-		}
 
 		if (!have_intermediate && [super_class instancesRespondToSelector:@selector(copyWithZone:)]) {
 			if (copyWithZone_signature[0] == '\0') {
 			iv = ((PyObjCInstanceVariable*)o);
 
 			if (iv->isOutlet) continue;
-			if (strcmp(iv->type, "@") != 0) continue;
+			if (strcmp(iv->type, "@") != 0 && strcmp(iv->type, @encode(PyObject*)) != 0) continue;
 
 			var = class_getInstanceVariable(objcClass, iv->name);
 			if (var == NULL) continue;
 			if (iv->isSlot) {
 				Py_XDECREF(*(PyObject**)(((char*)self) + 
 					var->ivar_offset));
+				(*(PyObject**)(((char*)self) + 
+					var->ivar_offset)) = NULL;
 			} else {
 				PyObjC_DURING
-					[*(id*)(((char*)self) + var->ivar_offset) release];
+					[*(id*)(((char*)self) + var->ivar_offset) autorelease];
 
 				PyObjC_HANDLER
 					NSLog(@"ignoring exception %@ in destructor",
 		}
 		if (arg_self != ((PyObjCSelector*)pymeth)->sel_self) {
 
-			printf("self[%p]:%s sel_self[%p]:%s\n",
-				arg_self, PyObject_REPR(arg_self),
-				((PyObjCSelector*)pymeth)->sel_self,
-				PyObject_REPR(((PyObjCSelector*)pymeth)->sel_self));
-
-
 			PyErr_SetString(PyExc_TypeError,
 				"PyObjC_CallPython called with 'self' and "
 				"a method bound to another object");