Skip to content

Commit 56606f3

Browse files
dpgeorgedpgeorge
committed
py: Implement delete for property and descriptors.
Without this patch deleting a property, or class with descriptor, will call the setter with a NULL value and lead to a crash.
1 parent 0528c5a commit 56606f3

2 files changed

Lines changed: 44 additions & 19 deletions

File tree

‎py/objtype.c‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -533,35 +533,59 @@ bool mp_obj_instance_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) {
533533
if (member[0] != MP_OBJ_NULL) {
534534
#if MICROPY_PY_BUILTINS_PROPERTY
535535
if (MP_OBJ_IS_TYPE(member[0], &mp_type_property)) {
536-
// attribute exists and is a property; delegate the store
536+
// attribute exists and is a property; delegate the store/delete
537537
// Note: This is an optimisation for code size and execution time.
538-
// The proper way to do it is have the functionality just below
539-
// in a __set__ method of the property object, and then it would
540-
// be called by the descriptor code down below. But that way
538+
// The proper way to do it is have the functionality just below in
539+
// a __set__/__delete__ method of the property object, and then it
540+
// would be called by the descriptor code down below. But that way
541541
// requires overhead for the nested mp_call's and overhead for
542542
// the code.
543543
const mp_obj_t *proxy = mp_obj_property_get(member[0]);
544-
if (proxy[1] == mp_const_none) {
545-
// TODO better error message?
546-
return false;
544+
mp_obj_t dest[2] = {self_in, value};
545+
if (value == MP_OBJ_NULL) {
546+
// delete attribute
547+
if (proxy[2] == mp_const_none) {
548+
// TODO better error message?
549+
return false;
550+
} else {
551+
mp_call_function_n_kw(proxy[2], 1, 0, dest);
552+
return true;
553+
}
547554
} else {
548-
mp_obj_t dest[2] = {self_in, value};
549-
mp_call_function_n_kw(proxy[1], 2, 0, dest);
550-
return true;
555+
// store attribute
556+
if (proxy[1] == mp_const_none) {
557+
// TODO better error message?
558+
return false;
559+
} else {
560+
mp_call_function_n_kw(proxy[1], 2, 0, dest);
561+
return true;
562+
}
551563
}
552564
}
553565
#endif
554566

555567
#if MICROPY_PY_DESCRIPTORS
556-
// found a class attribute; if it has a __set__ method then call it with the
557-
// class instance and value as arguments
558-
mp_obj_t attr_set_method[4];
559-
mp_load_method_maybe(member[0], MP_QSTR___set__, attr_set_method);
560-
if (attr_set_method[0] != MP_OBJ_NULL) {
561-
attr_set_method[2] = self_in;
562-
attr_set_method[3] = value;
563-
mp_call_method_n_kw(2, 0, attr_set_method);
564-
return true;
568+
// found a class attribute; if it has a __set__/__delete__ method then
569+
// call it with the class instance (and value) as arguments
570+
if (value == MP_OBJ_NULL) {
571+
// delete attribute
572+
mp_obj_t attr_delete_method[3];
573+
mp_load_method_maybe(member[0], MP_QSTR___delete__, attr_delete_method);
574+
if (attr_delete_method[0] != MP_OBJ_NULL) {
575+
attr_delete_method[2] = self_in;
576+
mp_call_method_n_kw(1, 0, attr_delete_method);
577+
return true;
578+
}
579+
} else {
580+
// store attribute
581+
mp_obj_t attr_set_method[4];
582+
mp_load_method_maybe(member[0], MP_QSTR___set__, attr_set_method);
583+
if (attr_set_method[0] != MP_OBJ_NULL) {
584+
attr_set_method[2] = self_in;
585+
attr_set_method[3] = value;
586+
mp_call_method_n_kw(2, 0, attr_set_method);
587+
return true;
588+
}
565589
}
566590
#endif
567591
}

‎py/qstrdefs.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ Q(__str__)
6969
#if MICROPY_PY_DESCRIPTORS
7070
Q(__get__)
7171
Q(__set__)
72+
Q(__delete__)
7273
#endif
7374
Q(__getattr__)
7475
Q(__del__)

0 commit comments

Comments
 (0)