diff options
| -rw-r--r-- | py/mpconfig.h | 8 | ||||
| -rw-r--r-- | py/objtype.c | 101 | ||||
| -rw-r--r-- | tests/basics/builtin_property_inherit.py | 53 | ||||
| -rw-r--r-- | tests/misc/non_compliant.py | 10 | ||||
| -rw-r--r-- | tests/misc/non_compliant.py.exp | 1 |
5 files changed, 161 insertions, 12 deletions
diff --git a/py/mpconfig.h b/py/mpconfig.h index 0ea087fb3..26c9cd92b 100644 --- a/py/mpconfig.h +++ b/py/mpconfig.h @@ -706,14 +706,16 @@ 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 +// Whether to support the descriptors __get__, __set__, __delete__ +// This costs some code size and makes load/store/delete of instance +// attributes slower for the classes that use this feature #ifndef MICROPY_PY_DESCRIPTORS #define MICROPY_PY_DESCRIPTORS (0) #endif // Whether to support class __delattr__ and __setattr__ methods -// This costs some code size and makes all del attrs and store attrs slow +// This costs some code size and makes store/delete of instance +// attributes slower for the classes that use this feature #ifndef MICROPY_PY_DELATTR_SETATTR #define MICROPY_PY_DELATTR_SETATTR (0) #endif diff --git a/py/objtype.c b/py/objtype.c index d7d0ed7ff..ef70dfce0 100644 --- a/py/objtype.c +++ b/py/objtype.c @@ -3,7 +3,7 @@ * * The MIT License (MIT) * - * Copyright (c) 2013, 2014 Damien P. George + * Copyright (c) 2013-2018 Damien P. George * Copyright (c) 2014-2016 Paul Sokolovsky * * Permission is hereby granted, free of charge, to any person obtaining a copy @@ -41,6 +41,12 @@ #define DEBUG_printf(...) (void)0 #endif +#define ENABLE_SPECIAL_ACCESSORS \ + (MICROPY_PY_DESCRIPTORS || MICROPY_PY_DELATTR_SETATTR || MICROPY_PY_BUILTINS_PROPERTY) + +#define TYPE_FLAG_IS_SUBCLASSED (0x0001) +#define TYPE_FLAG_HAS_SPECIAL_ACCESSORS (0x0002) + STATIC mp_obj_t static_class_method_make_new(const mp_obj_type_t *self_in, size_t n_args, size_t n_kw, const mp_obj_t *args); /******************************************************************************/ @@ -591,6 +597,11 @@ STATIC void mp_obj_instance_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *des mp_obj_class_lookup(&lookup, self->base.type); mp_obj_t member = dest[0]; if (member != MP_OBJ_NULL) { + if (!(self->base.type->flags & TYPE_FLAG_HAS_SPECIAL_ACCESSORS)) { + // Class doesn't have any special accessors to check so return straightaway + return; + } + #if MICROPY_PY_BUILTINS_PROPERTY if (MP_OBJ_IS_TYPE(member, &mp_type_property)) { // object member is a property; delegate the load to the property @@ -652,11 +663,15 @@ STATIC void mp_obj_instance_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *des STATIC bool mp_obj_instance_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) { mp_obj_instance_t *self = MP_OBJ_TO_PTR(self_in); + if (!(self->base.type->flags & TYPE_FLAG_HAS_SPECIAL_ACCESSORS)) { + // Class doesn't have any special accessors so skip their checks + goto skip_special_accessors; + } + #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, @@ -728,9 +743,9 @@ STATIC bool mp_obj_instance_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t val } #endif + #if MICROPY_PY_DELATTR_SETATTR if (value == MP_OBJ_NULL) { // delete attribute - #if MICROPY_PY_DELATTR_SETATTR // try __delattr__ first mp_obj_t attr_delattr_method[3]; mp_load_method_maybe(self_in, MP_QSTR___delattr__, attr_delattr_method); @@ -740,13 +755,8 @@ STATIC bool mp_obj_instance_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t val mp_call_method_n_kw(1, 0, attr_delattr_method); return true; } - #endif - - mp_map_elem_t *elem = mp_map_lookup(&self->members, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_REMOVE_IF_FOUND); - return elem != NULL; } else { // store attribute - #if MICROPY_PY_DELATTR_SETATTR // try __setattr__ first mp_obj_t attr_setattr_method[4]; mp_load_method_maybe(self_in, MP_QSTR___setattr__, attr_setattr_method); @@ -757,8 +767,17 @@ STATIC bool mp_obj_instance_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t val mp_call_method_n_kw(2, 0, attr_setattr_method); return true; } - #endif + } + #endif + +skip_special_accessors: + if (value == MP_OBJ_NULL) { + // delete attribute + mp_map_elem_t *elem = mp_map_lookup(&self->members, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_REMOVE_IF_FOUND); + return elem != NULL; + } else { + // store attribute mp_map_lookup(&self->members, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_ADD_IF_NOT_FOUND)->value = value; return true; } @@ -899,6 +918,34 @@ STATIC mp_int_t instance_get_buffer(mp_obj_t self_in, mp_buffer_info_t *bufinfo, // - there is a constant mp_obj_type_t (called mp_type_type) for the 'type' object // - creating a new class (a new type) creates a new mp_obj_type_t +#if ENABLE_SPECIAL_ACCESSORS +STATIC bool check_for_special_accessors(mp_obj_t key, mp_obj_t value) { + #if MICROPY_PY_DELATTR_SETATTR + if (key == MP_OBJ_NEW_QSTR(MP_QSTR___setattr__) || key == MP_OBJ_NEW_QSTR(MP_QSTR___delattr__)) { + return true; + } + #endif + #if MICROPY_PY_BUILTINS_PROPERTY + if (MP_OBJ_IS_TYPE(value, &mp_type_property)) { + return true; + } + #endif + #if MICROPY_PY_DESCRIPTORS + static const uint8_t to_check[] = { + MP_QSTR___get__, MP_QSTR___set__, MP_QSTR___delete__, + }; + for (size_t i = 0; i < MP_ARRAY_SIZE(to_check); ++i) { + mp_obj_t dest_temp[2]; + mp_load_method_protected(value, to_check[i], dest_temp, true); + if (dest_temp[0] != MP_OBJ_NULL) { + return true; + } + } + #endif + return false; +} +#endif + STATIC void type_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) { (void)kind; mp_obj_type_t *self = MP_OBJ_TO_PTR(self_in); @@ -985,6 +1032,19 @@ STATIC void type_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) { dest[0] = MP_OBJ_NULL; // indicate success } } else { + #if ENABLE_SPECIAL_ACCESSORS + // Check if we add any special accessor methods with this store + if (!(self->flags & TYPE_FLAG_HAS_SPECIAL_ACCESSORS)) { + if (check_for_special_accessors(MP_OBJ_NEW_QSTR(attr), dest[1])) { + if (self->flags & TYPE_FLAG_IS_SUBCLASSED) { + // This class is already subclassed so can't have special accessors added + mp_raise_msg(&mp_type_AttributeError, "can't add special method to already-subclassed class"); + } + self->flags |= TYPE_FLAG_HAS_SPECIAL_ACCESSORS; + } + } + #endif + // store attribute mp_map_elem_t *elem = mp_map_lookup(locals_map, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_ADD_IF_NOT_FOUND); elem->value = dest[1]; @@ -1016,6 +1076,7 @@ mp_obj_t mp_obj_new_type(qstr name, mp_obj_t bases_tuple, mp_obj_t locals_dict) // TODO might need to make a copy of locals_dict; at least that's how CPython does it // Basic validation of base classes + uint16_t base_flags = 0; size_t bases_len; mp_obj_t *bases_items; mp_obj_tuple_get(bases_tuple, &bases_len, &bases_items); @@ -1033,10 +1094,17 @@ mp_obj_t mp_obj_new_type(qstr name, mp_obj_t bases_tuple, mp_obj_t locals_dict) "type '%q' is not an acceptable base type", t->name)); } } + #if ENABLE_SPECIAL_ACCESSORS + if (mp_obj_is_instance_type(t)) { + t->flags |= TYPE_FLAG_IS_SUBCLASSED; + base_flags |= t->flags & TYPE_FLAG_HAS_SPECIAL_ACCESSORS; + } + #endif } mp_obj_type_t *o = m_new0(mp_obj_type_t, 1); o->base.type = &mp_type_type; + o->flags = base_flags; o->name = name; o->print = instance_print; o->make_new = mp_obj_instance_make_new; @@ -1069,6 +1137,21 @@ mp_obj_t mp_obj_new_type(qstr name, mp_obj_t bases_tuple, mp_obj_t locals_dict) o->locals_dict = MP_OBJ_TO_PTR(locals_dict); + #if ENABLE_SPECIAL_ACCESSORS + // Check if the class has any special accessor methods + if (!(o->flags & TYPE_FLAG_HAS_SPECIAL_ACCESSORS)) { + for (size_t i = 0; i < o->locals_dict->map.alloc; i++) { + if (MP_MAP_SLOT_IS_FILLED(&o->locals_dict->map, i)) { + const mp_map_elem_t *elem = &o->locals_dict->map.table[i]; + if (check_for_special_accessors(elem->key, elem->value)) { + o->flags |= TYPE_FLAG_HAS_SPECIAL_ACCESSORS; + break; + } + } + } + } + #endif + const mp_obj_type_t *native_base; size_t num_native_bases = instance_count_native_bases(o, &native_base); if (num_native_bases > 1) { diff --git a/tests/basics/builtin_property_inherit.py b/tests/basics/builtin_property_inherit.py new file mode 100644 index 000000000..27a091393 --- /dev/null +++ b/tests/basics/builtin_property_inherit.py @@ -0,0 +1,53 @@ +# test builtin property combined with inheritance +try: + property +except: + print("SKIP") + raise SystemExit + +# test property in a base class works for derived classes +class A: + @property + def x(self): + print('A x') + return 123 +class B(A): + pass +class C(B): + pass +class D: + pass +class E(C, D): + pass +print(A().x) +print(B().x) +print(C().x) +print(E().x) + +# test that we can add a property to base class after creation +class F: + pass +F.foo = property(lambda self: print('foo get')) +class G(F): + pass +F().foo +G().foo + +# should be able to add a property to already-subclassed class because it already has one +F.bar = property(lambda self: print('bar get')) +F().bar +G().bar + +# test case where class (H here) is already subclassed before adding attributes +class H: + pass +class I(H): + pass + +# should be able to add a normal member to already-subclassed class +H.val = 2 +print(I().val) + +# should be able to add a property to the derived class +I.baz = property(lambda self: print('baz get')) +I().baz diff --git a/tests/misc/non_compliant.py b/tests/misc/non_compliant.py index 31129f075..580583bf3 100644 --- a/tests/misc/non_compliant.py +++ b/tests/misc/non_compliant.py @@ -139,3 +139,13 @@ class A: class B(object, A): pass B().foo() + +# can't assign property (or other special accessors) to already-subclassed class +class A: + pass +class B(A): + pass +try: + A.bar = property() +except AttributeError: + print('AttributeError') diff --git a/tests/misc/non_compliant.py.exp b/tests/misc/non_compliant.py.exp index 061e3fcc8..3f15a1440 100644 --- a/tests/misc/non_compliant.py.exp +++ b/tests/misc/non_compliant.py.exp @@ -20,3 +20,4 @@ NotImplementedError AttributeError TypeError A.foo +AttributeError |