Implement general __get__/__set__ descriptors

property objects are no longer a special case and have been simplified
old-style native properties can probably all be removed as well, but, todo

src/builtins.c

@@ -7,6 +7,7 @@
 
 static KrkClass * Helper;
 static KrkClass * LicenseReader;
+static KrkClass * property;
 
 FUNC_SIG(list,__init__);
 FUNC_SIG(list,sort);
@@ -743,57 +744,70 @@ KRK_METHOD(LicenseReader,__call__,{
 	return krk_runtimeError(vm.exceptions->typeError, "unexpected error");
 })
 
-static KrkValue _property_init(int argc, KrkValue argv[], int hasKw) {
+#define IS_property(o) (krk_isInstanceOf(o,property))
-	if (argc != 2) {
+#define AS_property(o) (AS_INSTANCE(o))
-		return krk_runtimeError(vm.exceptions->notImplementedError, "Additional arguments to @property() are not supported.");
+KRK_METHOD(property,__init__,{
+	METHOD_TAKES_AT_LEAST(1);
+	METHOD_TAKES_AT_MOST(2); /* TODO fdel */
+	krk_attachNamedValue(&self->fields, "fget", argv[1]);
+
+	/* Try to attach doc */
+	if (IS_NATIVE(argv[1]) && AS_NATIVE(argv[1])->doc) {
+		krk_attachNamedValue(&self->fields, "__doc__",
+			OBJECT_VAL(krk_copyString(AS_NATIVE(argv[1])->doc, strlen(AS_NATIVE(argv[1])->doc))));
+	} else if (IS_CLOSURE(argv[1])) {
+		krk_attachNamedValue(&self->fields, "__doc__",
+			OBJECT_VAL(AS_CLOSURE(argv[1])->function->docstring));
 	}
 
-	return OBJECT_VAL(krk_newProperty(argv[1]));
+	/* Try to attach name */
+	if (IS_NATIVE(argv[1]) && AS_NATIVE(argv[1])->name) {
+		krk_attachNamedValue(&self->fields, "__name__",
+			OBJECT_VAL(krk_copyString(AS_NATIVE(argv[1])->name, strlen(AS_NATIVE(argv[1])->name))));
+	} else if (IS_CLOSURE(argv[1])) {
+		krk_attachNamedValue(&self->fields, "__name__",
+			OBJECT_VAL(AS_CLOSURE(argv[1])->function->name));
 	}
 
-static KrkValue _property_repr(int argc, KrkValue argv[], int hasKw) {
+	if (argc > 2)
-	if (argc != 1 || !IS_PROPERTY(argv[0])) return krk_runtimeError(vm.exceptions->typeError, "?");
+		krk_attachNamedValue(&self->fields, "fset", argv[2]);
-	struct StringBuilder sb = {0};
-	pushStringBuilderStr(&sb, "property(", 9);
 
-	KrkValue method = AS_PROPERTY(argv[0])->method;
+	return argv[0];
+})
 
-	if (IS_NATIVE(method)) {
+KRK_METHOD(property,setter,{
-		pushStringBuilderStr(&sb, (char*)AS_NATIVE(method)->name, strlen(AS_NATIVE(method)->name));
+	METHOD_TAKES_EXACTLY(1);
-	} else if (IS_CLOSURE(method)) {
+	krk_attachNamedValue(&self->fields, "fset", argv[1]);
-		pushStringBuilderStr(&sb, AS_CLOSURE(method)->function->name->chars, AS_CLOSURE(method)->function->name->length);
+	return argv[0]; /* Return the original property */
-	}
+})
 
-	pushStringBuilder(&sb,')');
+KRK_METHOD(property,__get__,{
-	return finishStringBuilder(&sb);
+	METHOD_TAKES_EXACTLY(1); /* the owner */
-}
 
-static KrkValue _property_doc(int argc, KrkValue argv[], int hasKw) {
+	KrkValue fget;
-	if (argc != 1 || !IS_PROPERTY(argv[0])) return krk_runtimeError(vm.exceptions->typeError, "?");
+	if (!krk_tableGet(&self->fields, OBJECT_VAL(S("fget")), &fget))
-	KrkValue method = AS_PROPERTY(argv[0])->method;
+		return krk_runtimeError(vm.exceptions->valueError, "property object is missing 'fget' attribute");
-	if (IS_NATIVE(method) && AS_NATIVE(method)->doc) {
-		return OBJECT_VAL(krk_copyString(AS_NATIVE(method)->doc, strlen(AS_NATIVE(method)->doc)));
-	} else if (IS_CLOSURE(method)) {
-		return OBJECT_VAL(AS_CLOSURE(method)->function->docstring);
-	}
-	return NONE_VAL();
-}
 
-static KrkValue _property_name(int argc, KrkValue argv[], int hasKw) {
+	krk_push(argv[1]);
-	if (argc != 1 || !IS_PROPERTY(argv[0])) return krk_runtimeError(vm.exceptions->typeError, "?");
+	return krk_callSimple(fget, 1, 0);
-	KrkValue method = AS_PROPERTY(argv[0])->method;
+})
-	if (IS_NATIVE(method) && AS_NATIVE(method)->name) {
-		return OBJECT_VAL(krk_copyString(AS_NATIVE(method)->name, strlen(AS_NATIVE(method)->name)));
-	} else if (IS_CLOSURE(method)) {
-		return OBJECT_VAL(AS_CLOSURE(method)->function->name);
-	}
-	return NONE_VAL();
-}
 
-static KrkValue _property_method(int argc, KrkValue argv[], int hasKw) {
+KRK_METHOD(property,__set__,{
-	if (argc != 1 || !IS_PROPERTY(argv[0])) return krk_runtimeError(vm.exceptions->typeError, "?");
+	METHOD_TAKES_EXACTLY(2); /* the owner and the value */
-	return AS_PROPERTY(argv[0])->method;
+
-}
+	krk_push(argv[1]);
+	krk_push(argv[2]);
+
+	KrkValue fset;
+	if (krk_tableGet(&self->fields, OBJECT_VAL(S("fset")), &fset))
+		return krk_callSimple(fset, 2, 0);
+
+	KrkValue fget;
+	if (krk_tableGet(&self->fields, OBJECT_VAL(S("fget")), &fget))
+		return krk_callSimple(fget, 2, 0);
+
+	return krk_runtimeError(vm.exceptions->attributeError, "attribute can not be set");
+})
 
 static KrkValue _id(int argc, KrkValue argv[], int hasKw) {
 	if (argc != 1) return krk_runtimeError(vm.exceptions->argumentError, "expected exactly one argument");
@@ -852,13 +866,12 @@ void _createAndBind_builtins(void) {
 		"attaching new properties to the @c \\__builtins__ instance."
 	);
 
-	krk_makeClass(vm.builtins, &vm.baseClasses->propertyClass, "property", vm.baseClasses->objectClass);
+	property = krk_makeClass(vm.builtins, &vm.baseClasses->propertyClass, "property", vm.baseClasses->objectClass);
-	krk_defineNative(&vm.baseClasses->propertyClass->methods, ".__init__", _property_init);
+	BIND_METHOD(property,__init__);
-	krk_defineNative(&vm.baseClasses->propertyClass->methods, ".__repr__", _property_repr);
+	BIND_METHOD(property,__get__);
-	krk_defineNative(&vm.baseClasses->propertyClass->methods, ":__doc__", _property_doc);
+	BIND_METHOD(property,__set__);
-	krk_defineNative(&vm.baseClasses->propertyClass->methods, ":__name__", _property_name);
+	BIND_METHOD(property,setter);
-	krk_defineNative(&vm.baseClasses->propertyClass->methods, ":__method__", _property_method);
+	krk_finalizeClass(property);
-	krk_finalizeClass(vm.baseClasses->propertyClass);
 
 	krk_makeClass(vm.builtins, &Helper, "Helper", vm.baseClasses->objectClass);
 	KRK_DOC(Helper,

src/memory.c

@@ -87,10 +87,6 @@ static void freeObject(KrkObj * object) {
 			FREE(KrkBytes, bytes);
 			break;
 		}
-		case OBJ_PROPERTY: {
-			FREE(KrkProperty, object);
-			break;
-		}
 	}
 }
 
@@ -181,11 +177,6 @@ static void blackenObject(KrkObj * object) {
 			markArray(&tuple->values);
 			break;
 		}
-		case OBJ_PROPERTY: {
-			KrkProperty * property = (KrkProperty *)object;
-			krk_markValue(property->method);
-			break;
-		}
 		case OBJ_NATIVE:
 		case OBJ_STRING:
 		case OBJ_BYTES:

src/object.c

@@ -278,12 +278,6 @@ KrkUpvalue * krk_newUpvalue(int slot) {
 	return upvalue;
 }
 
-KrkProperty * krk_newProperty(KrkValue method) {
-	KrkProperty * property = ALLOCATE_OBJECT(KrkProperty, OBJ_PROPERTY);
-	property->method = method;
-	return property;
-}
-
 KrkClass * krk_newClass(KrkString * name, KrkClass * baseClass) {
 	KrkClass * _class = ALLOCATE_OBJECT(KrkClass, OBJ_CLASS);
 	_class->name = name;

src/object.h

@@ -24,7 +24,6 @@ typedef enum {
 	OBJ_BOUND_METHOD,
 	OBJ_TUPLE,
 	OBJ_BYTES,
-	OBJ_PROPERTY,
 } ObjType;
 
 #undef KrkObj
@@ -182,6 +181,8 @@ typedef struct KrkClass {
 	KrkObj * _setslice;
 	KrkObj * _delslice;
 	KrkObj * _contains;
+	KrkObj * _descget;
+	KrkObj * _descset;
 } KrkClass;
 
 /**
@@ -240,19 +241,6 @@ typedef struct {
 	KrkValueArray values;
 } KrkTuple;
 
-/**
- * @brief Dynamic property.
- * @extends KrkObj
- *
- * A property is a value that is determined at runtime by a function and
- * for which modifications using the dot operator and an assignment result
- * in a function call.
- */
-typedef struct {
-	KrkObj obj;
-	KrkValue method;
-} KrkProperty;
-
 /**
  * @brief Mutable array of values.
  * @extends KrkInstance
@@ -381,7 +369,6 @@ extern KrkClass *       krk_newClass(KrkString * name, KrkClass * base);
 extern KrkInstance *    krk_newInstance(KrkClass * _class);
 extern KrkBoundMethod * krk_newBoundMethod(KrkValue receiver, KrkObj * method);
 extern KrkTuple *       krk_newTuple(size_t length);
-extern KrkProperty *    krk_newProperty(KrkValue method);
 extern KrkBytes *       krk_newBytes(size_t length, uint8_t * source);
 extern void krk_bytesUpdateHash(KrkBytes * bytes);
 extern void krk_tupleUpdateHash(KrkTuple * self);
@@ -414,8 +401,6 @@ extern void krk_tupleUpdateHash(KrkTuple * self);
 #define AS_BOUND_METHOD(value) ((KrkBoundMethod*)AS_OBJECT(value))
 #define IS_TUPLE(value)    isObjType(value, OBJ_TUPLE)
 #define AS_TUPLE(value)    ((KrkTuple *)AS_OBJECT(value))
-#define IS_PROPERTY(value) isObjType(value, OBJ_PROPERTY)
-#define AS_PROPERTY(value) ((KrkProperty *)AS_OBJECT(value))
 #define AS_LIST(value) (&((KrkList *)AS_OBJECT(value))->values)
 #define AS_DICT(value) (&((KrkDict *)AS_OBJECT(value))->entries)
 

src/threads.c

@@ -229,7 +229,7 @@ void _createAndBind_threadsMod(void) {
 	KRK_DOC(BIND_METHOD(Thread,start), "Start the thread. A thread may only be started once.");
 	KRK_DOC(BIND_METHOD(Thread,join), "Join the thread. Does not return until the thread finishes.");
 	KRK_DOC(BIND_METHOD(Thread,is_alive), "Query the status of the thread.");
-	KRK_DOC(AS_NATIVE(BIND_PROP(Thread,tid)->method), "The platform-specific thread identifier, if available. Usually an integer.");
+	KRK_DOC(BIND_PROP(Thread,tid), "The platform-specific thread identifier, if available. Usually an integer.");
 	krk_finalizeClass(Thread);
 
 	krk_makeClass(threadsModule, &Lock, "Lock", vm.baseClasses->objectClass);

src/vm.c

@@ -348,13 +348,14 @@ KrkNative * krk_defineNative(KrkTable * table, const char * name, NativeFn funct
  * be used with the "fields" table rather than the methods table. This will eventually replace
  * the ":field" option for defineNative().
  */
-KrkProperty * krk_defineNativeProperty(KrkTable * table, const char * name, NativeFn function) {
+KrkNative * krk_defineNativeProperty(KrkTable * table, const char * name, NativeFn function) {
 	KrkNative * func = krk_newNative(function, name, 1);
 	krk_push(OBJECT_VAL(func));
-	KrkProperty * property = krk_newProperty(krk_peek(0));
+	KrkInstance * property = krk_newInstance(vm.baseClasses->propertyClass);
 	krk_attachNamedObject(table, name, (KrkObj*)property);
+	krk_attachNamedObject(&property->fields, "fget", (KrkObj*)func);
 	krk_pop();
-	return property;
+	return func;
 }
 
 /**
@@ -410,6 +411,8 @@ void krk_finalizeClass(KrkClass * _class) {
 		{&_class->_getattr, METHOD_GETATTR},
 		{&_class->_dir, METHOD_DIR},
 		{&_class->_contains, METHOD_CONTAINS},
+		{&_class->_descget, METHOD_DESCGET},
+		{&_class->_descset, METHOD_DESCSET},
 		{NULL, 0},
 	};
 
@@ -481,8 +484,6 @@ inline KrkClass * krk_getType(KrkValue of) {
 					return vm.baseClasses->tupleClass;
 				case OBJ_BYTES:
 					return vm.baseClasses->bytesClass;
-				case OBJ_PROPERTY:
-					return vm.baseClasses->propertyClass;
 				case OBJ_INSTANCE:
 					return AS_INSTANCE(of)->_class;
 				default:
@@ -906,10 +907,6 @@ int krk_bindMethod(KrkClass * _class, KrkString * name) {
 	if (!_class) return 0;
 	if (IS_NATIVE(method) && ((KrkNative*)AS_OBJECT(method))->isMethod == 2) {
 		out = AS_NATIVE(method)->function(1, (KrkValue[]){krk_peek(0)}, 0);
-	} else if (IS_PROPERTY(method)) {
-		/* Need to push for property object */
-		krk_push(krk_peek(0));
-		out = krk_callSimple(AS_PROPERTY(method)->method, 1, 0);
 	} else if (IS_CLOSURE(method) && (AS_CLOSURE(method)->function->isClassMethod)) {
 		out = OBJECT_VAL(krk_newBoundMethod(OBJECT_VAL(_class), AS_OBJECT(method)));
 	} else if (IS_CLOSURE(method) && (AS_CLOSURE(method)->function->isStaticMethod)) {
@@ -917,6 +914,14 @@ int krk_bindMethod(KrkClass * _class, KrkString * name) {
 	} else if (IS_CLOSURE(method) || IS_NATIVE(method)) {
 		out = OBJECT_VAL(krk_newBoundMethod(krk_peek(0), AS_OBJECT(method)));
 	} else {
+		/* Does it have a descriptor __get__? */
+		KrkClass * type = krk_getType(method);
+		if (type->_descget) {
+			krk_push(method);
+			krk_swap(1);
+			krk_push(krk_callSimple(OBJECT_VAL(type->_descget), 2, 0));
+			return 1;
+		}
 		out = method;
 	}
 	krk_pop();
@@ -1186,6 +1191,9 @@ void krk_initVM(int flags) {
 		/* Attribute access */
 		_(METHOD_GETATTR, "__getattr__"),
 		_(METHOD_DIR, "__dir__"),
+		/* Descriptor methods */
+		_(METHOD_DESCGET, "__get__"),
+		_(METHOD_DESCSET, "__set__"),
 	#undef _
 	};
 	for (size_t i = 0; i < METHOD__MAX; ++i) {
@@ -1806,28 +1814,41 @@ static int valueDelProperty(KrkString * name) {
 	return 0;
 }
 
+static int trySetDescriptor(KrkValue owner, KrkString * name, KrkValue value) {
+	KrkClass * _class = krk_getType(owner);
+	KrkValue property;
+	while (_class) {
+		if (krk_tableGet(&_class->methods, OBJECT_VAL(name), &property)) break;
+		_class = _class->base;
+	}
+	if (_class) {
+		KrkClass * type = krk_getType(property);
+		if (type->_descset) {
+			/* Need to rearrange arguments */
+			krk_push(property); /* owner value property */
+			krk_swap(2);        /* property value owner */
+			krk_swap(1);        /* property owner value */
+			krk_push(krk_callSimple(OBJECT_VAL(type->_descset), 3, 0));
+			return 1;
+		}
+	}
+	return 0;
+}
+
 static int valueSetProperty(KrkString * name) {
 	KrkValue owner = krk_peek(1);
 	KrkValue value = krk_peek(0);
 	if (IS_INSTANCE(owner)) {
 		if (krk_tableSet(&AS_INSTANCE(owner)->fields, OBJECT_VAL(name), value)) {
-			KrkClass * _class = AS_INSTANCE(owner)->_class;
+			if (trySetDescriptor(owner, name, value)) {
-			KrkValue method;
-			while (_class) {
-				if (krk_tableGet(&_class->methods, OBJECT_VAL(name), &method)) break;
-				_class = _class->base;
-			}
-			if (_class && IS_PROPERTY(method)) {
 				krk_tableDelete(&AS_INSTANCE(owner)->fields, OBJECT_VAL(name));
-				krk_push(krk_callSimple(AS_PROPERTY(method)->method, 2, 0));
 				return 1;
 			}
 		}
 	} else if (IS_CLASS(owner)) {
 		krk_tableSet(&AS_CLASS(owner)->methods, OBJECT_VAL(name), value);
 	} else {
-		/* TODO: Setters for other things */
+		return (trySetDescriptor(owner,name,value));
-		return 0;
 	}
 	krk_swap(1);
 	krk_pop();

src/vm.h

@@ -88,6 +88,8 @@ typedef enum {
 	METHOD_SETSLICE,
 	METHOD_DELSLICE,
 	METHOD_CONTAINS,
+	METHOD_DESCGET,
+	METHOD_DESCSET,
 
 	METHOD__MAX,
 } KrkSpecialMethods;
@@ -423,7 +425,7 @@ extern KrkNative * krk_defineNative(KrkTable * table, const char * name, NativeF
  * @param func     Native function pointer to attach
  * @return A pointer to the property object created.
  */
-extern KrkProperty * krk_defineNativeProperty(KrkTable * table, const char * name, NativeFn func);
+extern KrkNative * krk_defineNativeProperty(KrkTable * table, const char * name, NativeFn func);
 
 /**
  * @brief Attach a value to an attribute table.

test/testDescriptor.krk

@@ -0,0 +1,41 @@
+
+def intDescriptor(inst, *args):
+    if args:
+        print("Setter called on",inst,"with value",args)
+    else:
+        print("Getter called on",inst)
+        return inst * inst
+
+int.foo = property(intDescriptor)
+
+print(2.foo)
+2.foo = 72
+
+# Now let's try an example straight from the Python docs
+
+class LoggedAgeAccess:
+    def __get__(self, obj, objtype=None):
+        let value = obj._age
+        print("Accessing 'age' of", obj.name)
+        return value
+    def __set__(self, obj, value):
+        print("Updating 'age' of", obj.name, "to", value)
+        obj._age = value
+
+class Person:
+    age = LoggedAgeAccess()
+    def __init__(self, name, age):
+        self.name = name
+        self.age = age
+    def birthday(self):
+        self.age += 1
+
+let mary = Person('Mary M', 30)
+let dave = Person('Dave D', 40)
+print(*(x for x in dir(mary) if x not in dir(type(mary))))
+print(*(x for x in dir(dave) if x not in dir(type(dave))))
+
+print(mary.age)
+mary.birthday()
+print(dave.age)
+

test/testDescriptor.krk.expect

@@ -0,0 +1,13 @@
+Getter called on 2
+4
+Setter called on 2 with value [72]
+Updating 'age' of Mary M to 30
+Updating 'age' of Dave D to 40
+_age name
+_age name
+Accessing 'age' of Mary M
+30
+Accessing 'age' of Mary M
+Updating 'age' of Mary M to 31
+Accessing 'age' of Dave D
+40

test/testSpecialDecorators.krk

@@ -27,3 +27,19 @@ print(f.bar)
 Foo.fromString("test")
 f.fromString("test")
 Bar.fromString("test")
+
+class Baz(object):
+    myBar = 42
+    @property
+    def bar(self):
+        print("I am a Python-style @property!")
+        return self.myBar
+    @bar.setter
+    def bar(self,value):
+        print("I am a Python-style @property's setter called with", value)
+        self.myBar = value
+
+let b = Baz()
+print(b.bar)
+b.bar = 0xDEADBEEF
+print(b.bar)

test/testSpecialDecorators.krk.expect

@@ -6,3 +6,8 @@ Called as a setter: [102]
 <class '__main__.Foo'> test
 <class '__main__.Foo'> test
 <class '__main__.Bar'> test
+I am a Python-style @property!
+42
+I am a Python-style @property's setter called with 3735928559
+I am a Python-style @property!
+3735928559

test/testSubclassPropertySuperCall.krk.expect

@@ -1,6 +1,6 @@
 True
 True
-['__class__', '__dir__', '__doc__', '__hash__', '__init__', '__method__', '__module__', '__name__', '__repr__', '__str__']
+['__class__', '__dir__', '__doc__', '__get__', '__hash__', '__init__', '__module__', '__name__', '__repr__', '__set__', '__str__', 'fget', 'setter']
 p retrieved from A
 {'a': 45}
 calling property from subclass