From 28fa84b4454ef253dfba5f1ceb023be2184a7271 Mon Sep 17 00:00:00 2001 From: stijn Date: Sat, 14 Feb 2015 18:43:54 +0100 Subject: [PATCH] py: Add optional support for descriptors' __get__ and __set__ methods. Disabled by default. Enabled on unix and windows ports. --- py/mpconfig.h | 6 +++ py/objtype.c | 85 ++++++++++++++++++++++++-------- py/qstrdefs.h | 4 ++ py/vm.c | 5 +- tests/basics/class_descriptor.py | 20 ++++++++ unix/mpconfigport.h | 1 + windows/mpconfigport.h | 1 + 7 files changed, 99 insertions(+), 23 deletions(-) create mode 100644 tests/basics/class_descriptor.py diff --git a/py/mpconfig.h b/py/mpconfig.h index 094d8d78a7..ae5e776256 100644 --- a/py/mpconfig.h +++ b/py/mpconfig.h @@ -364,6 +364,12 @@ typedef double mp_float_t; #define MICROPY_PY_FUNCTION_ATTRS (0) #endif +// Whether to support descriptors (__get__ and __set__) +// This costs some code size and makes all load attrs and store attrs slow +#ifndef MICROPY_PY_DESCRIPTORS +#define MICROPY_PY_DESCRIPTORS (0) +#endif + // Whether str object is proper unicode #ifndef MICROPY_PY_BUILTINS_STR_UNICODE #define MICROPY_PY_BUILTINS_STR_UNICODE (0) diff --git a/py/objtype.c b/py/objtype.c index f64e5054ad..c08a6357cf 100644 --- a/py/objtype.c +++ b/py/objtype.c @@ -461,20 +461,38 @@ void mp_obj_instance_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) { mp_obj_class_lookup(&lookup, self->base.type); mp_obj_t member = dest[0]; if (member != MP_OBJ_NULL) { -#if MICROPY_PY_BUILTINS_PROPERTY + #if MICROPY_PY_BUILTINS_PROPERTY if (MP_OBJ_IS_TYPE(member, &mp_type_property)) { - // object member is a property - // delegate the store to the property - // TODO should this be part of mp_convert_member_lookup? + // object member is a property; delegate the load to the property + // Note: This is an optimisation for code size and execution time. + // The proper way to do it is have the functionality just below + // in a __get__ method of the property object, and then it would + // be called by the descriptor code down below. But that way + // requires overhead for the nested mp_call's and overhead for + // the code. const mp_obj_t *proxy = mp_obj_property_get(member); if (proxy[0] == mp_const_none) { - // TODO + nlr_raise(mp_obj_new_exception_msg(&mp_type_AttributeError, "unreadable attribute")); } else { dest[0] = mp_call_function_n_kw(proxy[0], 1, 0, &self_in); - // TODO should we convert the returned value using mp_convert_member_lookup? } + return; } -#endif + #endif + + #if MICROPY_PY_DESCRIPTORS + // found a class attribute; if it has a __get__ method then call it with the + // class instance and class as arguments and return the result + // Note that this is functionally correct but very slow: each load_attr + // requires an extra mp_load_method_maybe to check for the __get__. + mp_obj_t attr_get_method[4]; + mp_load_method_maybe(member, MP_QSTR___get__, attr_get_method); + if (attr_get_method[0] != MP_OBJ_NULL) { + attr_get_method[2] = self_in; + attr_get_method[3] = mp_obj_get_type(self_in); + dest[0] = mp_call_method_n_kw(2, 0, attr_get_method); + } + #endif return; } @@ -495,9 +513,11 @@ void mp_obj_instance_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) { bool mp_obj_instance_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) { mp_obj_instance_t *self = self_in; -#if MICROPY_PY_BUILTINS_PROPERTY - // for property, we need to do a lookup first in the class dict - // this makes all stores slow... how to fix? + #if MICROPY_PY_BUILTINS_PROPERTY || MICROPY_PY_DESCRIPTORS + // With property and/or descriptors enabled we need to do a lookup + // first in the class dict for the attribute to see if the store should + // be delegated. + // Note: this makes all stores slow... how to fix? mp_obj_t member[2] = {MP_OBJ_NULL}; struct class_lookup_data lookup = { .obj = self, @@ -507,20 +527,43 @@ bool mp_obj_instance_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) { .is_type = false, }; mp_obj_class_lookup(&lookup, self->base.type); - if (member[0] != MP_OBJ_NULL && MP_OBJ_IS_TYPE(member[0], &mp_type_property)) { - // attribute already exists and is a property - // delegate the store to the property - const mp_obj_t *proxy = mp_obj_property_get(member[0]); - if (proxy[1] == mp_const_none) { - // TODO better error message - return false; - } else { - mp_obj_t dest[2] = {self_in, value}; - mp_call_function_n_kw(proxy[1], 2, 0, dest); + + if (member[0] != MP_OBJ_NULL) { + #if MICROPY_PY_BUILTINS_PROPERTY + if (MP_OBJ_IS_TYPE(member[0], &mp_type_property)) { + // attribute exists and is a property; delegate the store + // Note: This is an optimisation for code size and execution time. + // The proper way to do it is have the functionality just below + // in a __set__ method of the property object, and then it would + // be called by the descriptor code down below. But that way + // requires overhead for the nested mp_call's and overhead for + // the code. + const mp_obj_t *proxy = mp_obj_property_get(member[0]); + if (proxy[1] == mp_const_none) { + // TODO better error message? + return false; + } else { + mp_obj_t dest[2] = {self_in, value}; + mp_call_function_n_kw(proxy[1], 2, 0, dest); + return true; + } + } + #endif + + #if MICROPY_PY_DESCRIPTORS + // found a class attribute; if it has a __set__ method then call it with the + // class instance and value as arguments + mp_obj_t attr_set_method[4]; + mp_load_method_maybe(member[0], MP_QSTR___set__, attr_set_method); + if (attr_set_method[0] != MP_OBJ_NULL) { + attr_set_method[2] = self_in; + attr_set_method[3] = value; + mp_call_method_n_kw(2, 0, attr_set_method); return true; } + #endif } -#endif + #endif if (value == MP_OBJ_NULL) { // delete attribute diff --git a/py/qstrdefs.h b/py/qstrdefs.h index dc1f082da3..3c4106e853 100644 --- a/py/qstrdefs.h +++ b/py/qstrdefs.h @@ -66,6 +66,10 @@ Q(__add__) Q(__sub__) Q(__repr__) Q(__str__) +#if MICROPY_PY_DESCRIPTORS +Q(__get__) +Q(__set__) +#endif Q(__getattr__) Q(__del__) Q(__call__) diff --git a/py/vm.c b/py/vm.c index ee218694c4..29a9e4f6d6 100644 --- a/py/vm.c +++ b/py/vm.c @@ -393,8 +393,9 @@ dispatch_loop: DISPATCH(); } #else - // This caching code works with MICROPY_PY_BUILTINS_PROPERTY enabled because - // if the attr exists in self->members then it can't be a property. A + // This caching code works with MICROPY_PY_BUILTINS_PROPERTY and/or + // MICROPY_PY_DESCRIPTORS enabled because if the attr exists in + // self->members then it can't be a property or have descriptors. A // consequence of this is that we can't use MP_MAP_LOOKUP_ADD_IF_NOT_FOUND // in the fast-path below, because that store could override a property. ENTRY(MP_BC_STORE_ATTR): { diff --git a/tests/basics/class_descriptor.py b/tests/basics/class_descriptor.py new file mode 100644 index 0000000000..3f4b200bc8 --- /dev/null +++ b/tests/basics/class_descriptor.py @@ -0,0 +1,20 @@ +class Descriptor: + def __get__(self, obj, cls): + print(type(obj) is Main) + print(cls is Main) + return 'result' + + def __set__(self, obj, val): + print(type(obj) is Main) + print(val) + +class Main: + Forward = Descriptor() + +m = Main() +r = m.Forward +if 'Descriptor' in repr(r.__class__): + print('SKIP') +else: + print(r) + m.Forward = 'a' diff --git a/unix/mpconfigport.h b/unix/mpconfigport.h index fd8e522679..014272e7ae 100644 --- a/unix/mpconfigport.h +++ b/unix/mpconfigport.h @@ -58,6 +58,7 @@ #define MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE (1) #define MICROPY_CAN_OVERRIDE_BUILTINS (1) #define MICROPY_PY_FUNCTION_ATTRS (1) +#define MICROPY_PY_DESCRIPTORS (1) #define MICROPY_PY_BUILTINS_STR_UNICODE (1) #define MICROPY_PY_BUILTINS_MEMORYVIEW (1) #define MICROPY_PY_BUILTINS_FROZENSET (1) diff --git a/windows/mpconfigport.h b/windows/mpconfigport.h index 6e342f0d3d..01f8775fab 100644 --- a/windows/mpconfigport.h +++ b/windows/mpconfigport.h @@ -51,6 +51,7 @@ #define MICROPY_STREAMS_NON_BLOCK (1) #define MICROPY_OPT_COMPUTED_GOTO (0) #define MICROPY_CAN_OVERRIDE_BUILTINS (1) +#define MICROPY_PY_DESCRIPTORS (1) #define MICROPY_PY_BUILTINS_STR_UNICODE (1) #define MICROPY_PY_BUILTINS_MEMORYVIEW (1) #define MICROPY_PY_BUILTINS_FROZENSET (1)