Skip to content

Commit f49ba1b

Browse files
dpgeorgedpgeorge
committed
Improve method lookup in mp_obj_class_lookup.
Now searches both locals_dict and methods. Partly addresses Issue adafruit#145.
1 parent 1d6fc94 commit f49ba1b

3 files changed

Lines changed: 92 additions & 69 deletions

File tree

‎py/objtype.c‎

Lines changed: 73 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,26 @@ static mp_obj_t mp_obj_new_class(mp_obj_t class) {
2828
return o;
2929
}
3030

31-
static mp_map_elem_t *mp_obj_class_lookup(const mp_obj_type_t *type, qstr attr, mp_map_lookup_kind_t lookup_kind) {
31+
// will return MP_OBJ_NULL if not found
32+
static mp_obj_t mp_obj_class_lookup(const mp_obj_type_t *type, qstr attr) {
3233
for (;;) {
33-
if (type->locals_dict == NULL) {
34-
return NULL;
35-
}
36-
assert(MP_OBJ_IS_TYPE(type->locals_dict, &dict_type)); // Micro Python restriction, for now
37-
mp_map_t *locals_map = ((void*)type->locals_dict + sizeof(mp_obj_base_t)); // XXX hack to get map object from dict object
38-
mp_map_elem_t *elem = mp_map_lookup(locals_map, MP_OBJ_NEW_QSTR(attr), lookup_kind);
39-
if (elem != NULL) {
40-
return elem;
34+
if (type->locals_dict != NULL) {
35+
// search locals_dict (the dynamically created set of methods/attributes)
36+
37+
assert(MP_OBJ_IS_TYPE(type->locals_dict, &dict_type)); // Micro Python restriction, for now
38+
mp_map_t *locals_map = ((void*)type->locals_dict + sizeof(mp_obj_base_t)); // XXX hack to get map object from dict object
39+
mp_map_elem_t *elem = mp_map_lookup(locals_map, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP);
40+
if (elem != NULL) {
41+
return elem->value;
42+
}
43+
} else if (type->methods != NULL) {
44+
// search methods (the const set of methods)
45+
46+
for (const mp_method_t *meth = type->methods; meth->name != NULL; meth++) {
47+
if (strcmp(meth->name, qstr_str(attr)) == 0) {
48+
return (mp_obj_t)meth->fun;
49+
}
50+
}
4151
}
4252

4353
// attribute not found, keep searching base classes
@@ -55,9 +65,9 @@ static mp_map_elem_t *mp_obj_class_lookup(const mp_obj_type_t *type, qstr attr,
5565
}
5666
for (uint i = 0; i < len - 1; i++) {
5767
assert(MP_OBJ_IS_TYPE(items[i], &mp_const_type));
58-
elem = mp_obj_class_lookup((mp_obj_type_t*)items[i], attr, lookup_kind);
59-
if (elem != NULL) {
60-
return elem;
68+
mp_obj_t obj = mp_obj_class_lookup((mp_obj_type_t*)items[i], attr);
69+
if (obj != MP_OBJ_NULL) {
70+
return obj;
6171
}
6272
}
6373

@@ -78,18 +88,18 @@ static mp_obj_t class_make_new(mp_obj_t self_in, uint n_args, uint n_kw, const m
7888
mp_obj_t o = mp_obj_new_class(self_in);
7989

8090
// look for __init__ function
81-
mp_map_elem_t *init_fn = mp_obj_class_lookup(self, MP_QSTR___init__, MP_MAP_LOOKUP);
91+
mp_obj_t init_fn = mp_obj_class_lookup(self, MP_QSTR___init__);
8292

83-
if (init_fn != NULL) {
93+
if (init_fn != MP_OBJ_NULL) {
8494
// call __init__ function
8595
mp_obj_t init_ret;
8696
if (n_args == 0 && n_kw == 0) {
87-
init_ret = rt_call_function_n_kw(init_fn->value, 1, 0, (mp_obj_t*)&o);
97+
init_ret = rt_call_function_n_kw(init_fn, 1, 0, (mp_obj_t*)&o);
8898
} else {
8999
mp_obj_t *args2 = m_new(mp_obj_t, 1 + n_args + 2 * n_kw);
90100
args2[0] = o;
91101
memcpy(args2 + 1, args, (n_args + 2 * n_kw) * sizeof(mp_obj_t));
92-
init_ret = rt_call_function_n_kw(init_fn->value, n_args + 1, n_kw, args2);
102+
init_ret = rt_call_function_n_kw(init_fn, n_args + 1, n_kw, args2);
93103
m_del(mp_obj_t, args2, 1 + n_args + 2 * n_kw);
94104
}
95105
if (init_ret != mp_const_none) {
@@ -156,9 +166,9 @@ static mp_obj_t class_binary_op(int op, mp_obj_t lhs_in, mp_obj_t rhs_in) {
156166
if (op_name == NULL) {
157167
return MP_OBJ_NULL;
158168
}
159-
mp_map_elem_t *elem = mp_obj_class_lookup(lhs->base.type, qstr_from_str_static(op_name), MP_MAP_LOOKUP);
160-
if (elem != NULL) {
161-
return rt_call_function_2(elem->value, lhs_in, rhs_in);
169+
mp_obj_t member = mp_obj_class_lookup(lhs->base.type, qstr_from_str_static(op_name));
170+
if (member != MP_OBJ_NULL) {
171+
return rt_call_function_2(member, lhs_in, rhs_in);
162172
} else {
163173
return MP_OBJ_NULL;
164174
}
@@ -173,41 +183,49 @@ static void class_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) {
173183
dest[0] = elem->value;
174184
return;
175185
}
176-
elem = mp_obj_class_lookup(self->base.type, attr, MP_MAP_LOOKUP);
177-
if (elem != NULL) {
178-
if (mp_obj_is_callable(elem->value)) {
186+
mp_obj_t member = mp_obj_class_lookup(self->base.type, attr);
187+
if (member != MP_OBJ_NULL) {
188+
if (mp_obj_is_callable(member)) {
179189
// class member is callable so build a bound method
180-
dest[0] = elem->value;
181-
dest[1] = self_in;
190+
// check if the methods are functions, static or class methods
191+
// see http://docs.python.org/3.3/howto/descriptor.html
192+
// TODO check that this is the correct place to have this logic
193+
if (MP_OBJ_IS_TYPE(member, &mp_type_staticmethod)) {
194+
// return just the function
195+
dest[0] = ((mp_obj_staticmethod_t*)member)->fun;
196+
} else if (MP_OBJ_IS_TYPE(member, &mp_type_classmethod)) {
197+
// return a bound method, with self being the type of this object
198+
dest[0] = ((mp_obj_classmethod_t*)member)->fun;
199+
dest[1] = mp_obj_get_type(self_in);
200+
} else {
201+
// return a bound method, with self being this object
202+
dest[0] = member;
203+
dest[1] = self_in;
204+
}
182205
return;
183206
} else {
184207
// class member is a value, so just return that value
185-
dest[0] = elem->value;
208+
dest[0] = member;
186209
return;
187210
}
188211
}
189212
}
190213

191214
static bool class_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) {
192-
// logic: look in class locals (no add) then obj members (add) (TODO check this against CPython)
193215
mp_obj_class_t *self = self_in;
194-
mp_map_elem_t *elem = mp_obj_class_lookup(self->base.type, attr, MP_MAP_LOOKUP);
195-
if (elem != NULL) {
196-
elem->value = value;
197-
} else {
198-
mp_map_lookup(&self->members, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_ADD_IF_NOT_FOUND)->value = value;
199-
}
216+
mp_map_lookup(&self->members, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_ADD_IF_NOT_FOUND)->value = value;
200217
return true;
201218
}
202219

203220
bool class_store_item(mp_obj_t self_in, mp_obj_t index, mp_obj_t value) {
204221
mp_obj_class_t *self = self_in;
205-
mp_map_elem_t *elem = mp_obj_class_lookup(self->base.type, qstr_from_str_static("__setitem__"), MP_MAP_LOOKUP);
206-
if (elem != NULL) {
222+
mp_obj_t member = mp_obj_class_lookup(self->base.type, qstr_from_str_static("__setitem__"));
223+
if (member != MP_OBJ_NULL) {
207224
mp_obj_t args[3] = {self_in, index, value};
208-
return rt_call_function_n_kw(elem->value, 3, 0, args);
225+
rt_call_function_n_kw(member, 3, 0, args);
226+
return true;
209227
} else {
210-
return MP_OBJ_NULL;
228+
return false;
211229
}
212230
}
213231

@@ -260,34 +278,21 @@ static mp_obj_t type_call(mp_obj_t self_in, uint n_args, uint n_kw, const mp_obj
260278
static void type_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) {
261279
assert(MP_OBJ_IS_TYPE(self_in, &mp_const_type));
262280
mp_obj_type_t *self = self_in;
263-
mp_map_elem_t *elem = mp_obj_class_lookup(self, attr, MP_MAP_LOOKUP);
264-
if (elem != NULL) {
265-
dest[0] = elem->value;
266-
return;
267-
}
268-
269-
// generic method lookup
270-
// this is a lookup in the class itself (ie not the classes type or instance)
271-
const mp_method_t *meth = self->methods;
272-
if (meth != NULL) {
273-
for (; meth->name != NULL; meth++) {
274-
if (strcmp(meth->name, qstr_str(attr)) == 0) {
275-
// check if the methods are functions, static or class methods
276-
// see http://docs.python.org/3.3/howto/descriptor.html
277-
if (MP_OBJ_IS_TYPE(meth->fun, &mp_type_staticmethod)) {
278-
// return just the function
279-
dest[0] = ((mp_obj_staticmethod_t*)meth->fun)->fun;
280-
} else if (MP_OBJ_IS_TYPE(meth->fun, &mp_type_classmethod)) {
281-
// return a bound method, with self being this class
282-
dest[0] = ((mp_obj_classmethod_t*)meth->fun)->fun;
283-
dest[1] = self_in;
284-
} else {
285-
// return just the function
286-
// TODO need to wrap in a type check for the first argument; eg list.append(1,1) needs to throw an exception
287-
dest[0] = (mp_obj_t)meth->fun;
288-
}
289-
return;
290-
}
281+
mp_obj_t member = mp_obj_class_lookup(self, attr);
282+
if (member != MP_OBJ_NULL) {
283+
// check if the methods are functions, static or class methods
284+
// see http://docs.python.org/3.3/howto/descriptor.html
285+
if (MP_OBJ_IS_TYPE(member, &mp_type_staticmethod)) {
286+
// return just the function
287+
dest[0] = ((mp_obj_staticmethod_t*)member)->fun;
288+
} else if (MP_OBJ_IS_TYPE(member, &mp_type_classmethod)) {
289+
// return a bound method, with self being this class
290+
dest[0] = ((mp_obj_classmethod_t*)member)->fun;
291+
dest[1] = self_in;
292+
} else {
293+
// return just the function
294+
// TODO need to wrap in a type check for the first argument; eg list.append(1,1) needs to throw an exception
295+
dest[0] = (mp_obj_t)member;
291296
}
292297
}
293298
}
@@ -298,9 +303,10 @@ static bool type_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) {
298303

299304
// TODO CPython allows STORE_ATTR to a class, but is this the correct implementation?
300305

301-
mp_map_elem_t *elem = mp_obj_class_lookup(self, attr, MP_MAP_LOOKUP_ADD_IF_NOT_FOUND);
302-
if (elem != NULL) {
303-
elem->value = value;
306+
if (self->locals_dict != NULL) {
307+
assert(MP_OBJ_IS_TYPE(self->locals_dict, &dict_type)); // Micro Python restriction, for now
308+
mp_map_t *locals_map = ((void*)self->locals_dict + sizeof(mp_obj_base_t)); // XXX hack to get map object from dict object
309+
mp_map_lookup(locals_map, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_ADD_IF_NOT_FOUND)->value = value;
304310
return true;
305311
} else {
306312
return false;

‎py/runtime.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -820,8 +820,8 @@ void rt_load_method(mp_obj_t base, qstr attr, mp_obj_t *dest) {
820820
if (attr == MP_QSTR___next__ && type->iternext != NULL) {
821821
dest[0] = (mp_obj_t)&mp_builtin_next_obj;
822822
dest[1] = base;
823-
} else {
824-
// generic method lookup
823+
} else if (type->load_attr == NULL) {
824+
// generic method lookup if type didn't provide a specific one
825825
// this is a lookup in the object (ie not class or type)
826826
const mp_method_t *meth = type->methods;
827827
if (meth != NULL) {

‎tests/basics/tests/class_store.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# store to class vs instance
2+
3+
class C:
4+
pass
5+
6+
c = C()
7+
c.x = 1
8+
print(c.x)
9+
C.x = 2
10+
C.y = 3
11+
print(c.x, c.y)
12+
print(C.x, C.y)
13+
print(C().x, C().y)
14+
c = C()
15+
print(c.x)
16+
c.x = 4
17+
print(c.x)

0 commit comments

Comments
 (0)