From 6b3f8de63bb0a89c744877d596b9148d1d21b156 Mon Sep 17 00:00:00 2001 From: "K. Lange" Date: Wed, 10 Mar 2021 20:24:12 +0900 Subject: [PATCH] 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 | 115 ++++++++++-------- src/memory.c | 9 -- src/object.c | 6 - src/object.h | 19 +-- src/threads.c | 2 +- src/vm.c | 59 ++++++--- src/vm.h | 4 +- test/testDescriptor.krk | 41 +++++++ test/testDescriptor.krk.expect | 13 ++ test/testSpecialDecorators.krk | 16 +++ test/testSpecialDecorators.krk.expect | 5 + test/testSubclassPropertySuperCall.krk.expect | 2 +- 12 files changed, 186 insertions(+), 105 deletions(-) create mode 100644 test/testDescriptor.krk create mode 100644 test/testDescriptor.krk.expect diff --git a/src/builtins.c b/src/builtins.c index 9fea894..f7d0a59 100644 --- a/src/builtins.c +++ b/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) { - if (argc != 2) { - return krk_runtimeError(vm.exceptions->notImplementedError, "Additional arguments to @property() are not supported."); +#define IS_property(o) (krk_isInstanceOf(o,property)) +#define AS_property(o) (AS_INSTANCE(o)) +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])); -} - -static KrkValue _property_repr(int argc, KrkValue argv[], int hasKw) { - if (argc != 1 || !IS_PROPERTY(argv[0])) return krk_runtimeError(vm.exceptions->typeError, "?"); - struct StringBuilder sb = {0}; - pushStringBuilderStr(&sb, "property(", 9); - - KrkValue method = AS_PROPERTY(argv[0])->method; - - if (IS_NATIVE(method)) { - pushStringBuilderStr(&sb, (char*)AS_NATIVE(method)->name, strlen(AS_NATIVE(method)->name)); - } else if (IS_CLOSURE(method)) { - pushStringBuilderStr(&sb, AS_CLOSURE(method)->function->name->chars, AS_CLOSURE(method)->function->name->length); + /* 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)); } - pushStringBuilder(&sb,')'); - return finishStringBuilder(&sb); -} + if (argc > 2) + krk_attachNamedValue(&self->fields, "fset", argv[2]); -static KrkValue _property_doc(int argc, KrkValue argv[], int hasKw) { - if (argc != 1 || !IS_PROPERTY(argv[0])) return krk_runtimeError(vm.exceptions->typeError, "?"); - KrkValue method = AS_PROPERTY(argv[0])->method; - 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(); -} + return argv[0]; +}) -static KrkValue _property_name(int argc, KrkValue argv[], int hasKw) { - if (argc != 1 || !IS_PROPERTY(argv[0])) return krk_runtimeError(vm.exceptions->typeError, "?"); - 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(); -} +KRK_METHOD(property,setter,{ + METHOD_TAKES_EXACTLY(1); + krk_attachNamedValue(&self->fields, "fset", argv[1]); + return argv[0]; /* Return the original property */ +}) -static KrkValue _property_method(int argc, KrkValue argv[], int hasKw) { - if (argc != 1 || !IS_PROPERTY(argv[0])) return krk_runtimeError(vm.exceptions->typeError, "?"); - return AS_PROPERTY(argv[0])->method; -} +KRK_METHOD(property,__get__,{ + METHOD_TAKES_EXACTLY(1); /* the owner */ + + KrkValue fget; + if (!krk_tableGet(&self->fields, OBJECT_VAL(S("fget")), &fget)) + return krk_runtimeError(vm.exceptions->valueError, "property object is missing 'fget' attribute"); + + krk_push(argv[1]); + return krk_callSimple(fget, 1, 0); +}) + +KRK_METHOD(property,__set__,{ + METHOD_TAKES_EXACTLY(2); /* the owner and the value */ + + 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); - krk_defineNative(&vm.baseClasses->propertyClass->methods, ".__init__", _property_init); - krk_defineNative(&vm.baseClasses->propertyClass->methods, ".__repr__", _property_repr); - krk_defineNative(&vm.baseClasses->propertyClass->methods, ":__doc__", _property_doc); - krk_defineNative(&vm.baseClasses->propertyClass->methods, ":__name__", _property_name); - krk_defineNative(&vm.baseClasses->propertyClass->methods, ":__method__", _property_method); - krk_finalizeClass(vm.baseClasses->propertyClass); + property = krk_makeClass(vm.builtins, &vm.baseClasses->propertyClass, "property", vm.baseClasses->objectClass); + BIND_METHOD(property,__init__); + BIND_METHOD(property,__get__); + BIND_METHOD(property,__set__); + BIND_METHOD(property,setter); + krk_finalizeClass(property); krk_makeClass(vm.builtins, &Helper, "Helper", vm.baseClasses->objectClass); KRK_DOC(Helper, diff --git a/src/memory.c b/src/memory.c index 8a8d430..773eaef 100644 --- a/src/memory.c +++ b/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: diff --git a/src/object.c b/src/object.c index 8029b77..e9fe720 100644 --- a/src/object.c +++ b/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; diff --git a/src/object.h b/src/object.h index 74beb7e..835adaf 100644 --- a/src/object.h +++ b/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) diff --git a/src/threads.c b/src/threads.c index cd9e433..29bfbcf 100644 --- a/src/threads.c +++ b/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); diff --git a/src/vm.c b/src/vm.c index eaa4052..5d6c179 100644 --- a/src/vm.c +++ b/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; - KrkValue method; - while (_class) { - if (krk_tableGet(&_class->methods, OBJECT_VAL(name), &method)) break; - _class = _class->base; - } - if (_class && IS_PROPERTY(method)) { + if (trySetDescriptor(owner, name, value)) { 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 0; + return (trySetDescriptor(owner,name,value)); } krk_swap(1); krk_pop(); diff --git a/src/vm.h b/src/vm.h index 5cfc799..a73c28c 100644 --- a/src/vm.h +++ b/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. diff --git a/test/testDescriptor.krk b/test/testDescriptor.krk new file mode 100644 index 0000000..21c8017 --- /dev/null +++ b/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) + diff --git a/test/testDescriptor.krk.expect b/test/testDescriptor.krk.expect new file mode 100644 index 0000000..8a50919 --- /dev/null +++ b/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 diff --git a/test/testSpecialDecorators.krk b/test/testSpecialDecorators.krk index 64b9d64..2367a8d 100644 --- a/test/testSpecialDecorators.krk +++ b/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) diff --git a/test/testSpecialDecorators.krk.expect b/test/testSpecialDecorators.krk.expect index e3d9f88..e581331 100644 --- a/test/testSpecialDecorators.krk.expect +++ b/test/testSpecialDecorators.krk.expect @@ -6,3 +6,8 @@ Called as a setter: [102] test test 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 diff --git a/test/testSubclassPropertySuperCall.krk.expect b/test/testSubclassPropertySuperCall.krk.expect index 1aadf21..80e91fb 100644 --- a/test/testSubclassPropertySuperCall.krk.expect +++ b/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