From 332d83343fb3ef5d2b94b4f058aa53fd0493779e Mon Sep 17 00:00:00 2001 From: Damien George Date: Sun, 28 Jun 2020 09:39:20 +1000 Subject: [PATCH] py: Rework mp_convert_member_lookup to properly handle built-ins. This commit fixes lookups of class members to make it so that built-in functions that are used as methods/functions of a class work correctly. The mp_convert_member_lookup() function is pretty much completely changed by this commit, but for the most part it's just reorganised and the indenting changed. The functional changes are: - staticmethod and classmethod checks moved to later in the if-logic, because they are less common and so should be checked after the more common cases. - The explicit mp_obj_is_type(member, &mp_type_type) check is removed because it's now subsumed by other, more general tests in this function. - MP_TYPE_FLAG_BINDS_SELF and MP_TYPE_FLAG_BUILTIN_FUN type flags added to make the checks in this function much simpler (now they just test this bit in type->flags). - An extra check is made for mp_obj_is_instance_type(type) to fix lookup of built-in functions. Fixes #1326 and #6198. Signed-off-by: Damien George --- py/obj.h | 4 ++ py/objclosure.c | 1 + py/objfun.c | 8 ++++ py/objgenerator.c | 2 + py/runtime.c | 82 ++++++++++++++++++--------------- tests/basics/class_bind_self.py | 9 +++- 6 files changed, 67 insertions(+), 39 deletions(-) diff --git a/py/obj.h b/py/obj.h index 6bec57c35..f621b1dad 100644 --- a/py/obj.h +++ b/py/obj.h @@ -473,11 +473,15 @@ typedef mp_obj_t (*mp_fun_kw_t)(size_t n, const mp_obj_t *, mp_map_t *); // then the type may check for equality against a different type. // If MP_TYPE_FLAG_EQ_HAS_NEQ_TEST is clear then the type only implements the __eq__ // operator and not the __ne__ operator. If it's set then __ne__ may be implemented. +// If MP_TYPE_FLAG_BINDS_SELF is set then the type as a method binds self as the first arg. +// If MP_TYPE_FLAG_BUILTIN_FUN is set then the type is a built-in function type. #define MP_TYPE_FLAG_IS_SUBCLASSED (0x0001) #define MP_TYPE_FLAG_HAS_SPECIAL_ACCESSORS (0x0002) #define MP_TYPE_FLAG_EQ_NOT_REFLEXIVE (0x0004) #define MP_TYPE_FLAG_EQ_CHECKS_OTHER_TYPE (0x0008) #define MP_TYPE_FLAG_EQ_HAS_NEQ_TEST (0x0010) +#define MP_TYPE_FLAG_BINDS_SELF (0x0020) +#define MP_TYPE_FLAG_BUILTIN_FUN (0x0040) typedef enum { PRINT_STR = 0, diff --git a/py/objclosure.c b/py/objclosure.c index f5038ffc4..9a8c7631c 100644 --- a/py/objclosure.c +++ b/py/objclosure.c @@ -80,6 +80,7 @@ STATIC void closure_print(const mp_print_t *print, mp_obj_t o_in, mp_print_kind_ const mp_obj_type_t closure_type = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF, .name = MP_QSTR_closure, #if MICROPY_ERROR_REPORTING == MICROPY_ERROR_REPORTING_DETAILED .print = closure_print, diff --git a/py/objfun.c b/py/objfun.c index f9a6b4ebc..052f4b1ce 100644 --- a/py/objfun.c +++ b/py/objfun.c @@ -58,6 +58,7 @@ STATIC mp_obj_t fun_builtin_0_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const mp_obj_type_t mp_type_fun_builtin_0 = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF | MP_TYPE_FLAG_BUILTIN_FUN, .name = MP_QSTR_function, .call = fun_builtin_0_call, .unary_op = mp_generic_unary_op, @@ -72,6 +73,7 @@ STATIC mp_obj_t fun_builtin_1_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const mp_obj_type_t mp_type_fun_builtin_1 = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF | MP_TYPE_FLAG_BUILTIN_FUN, .name = MP_QSTR_function, .call = fun_builtin_1_call, .unary_op = mp_generic_unary_op, @@ -86,6 +88,7 @@ STATIC mp_obj_t fun_builtin_2_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const mp_obj_type_t mp_type_fun_builtin_2 = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF | MP_TYPE_FLAG_BUILTIN_FUN, .name = MP_QSTR_function, .call = fun_builtin_2_call, .unary_op = mp_generic_unary_op, @@ -100,6 +103,7 @@ STATIC mp_obj_t fun_builtin_3_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const mp_obj_type_t mp_type_fun_builtin_3 = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF | MP_TYPE_FLAG_BUILTIN_FUN, .name = MP_QSTR_function, .call = fun_builtin_3_call, .unary_op = mp_generic_unary_op, @@ -130,6 +134,7 @@ STATIC mp_obj_t fun_builtin_var_call(mp_obj_t self_in, size_t n_args, size_t n_k const mp_obj_type_t mp_type_fun_builtin_var = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF | MP_TYPE_FLAG_BUILTIN_FUN, .name = MP_QSTR_function, .call = fun_builtin_var_call, .unary_op = mp_generic_unary_op, @@ -355,6 +360,7 @@ void mp_obj_fun_bc_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) { const mp_obj_type_t mp_type_fun_bc = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF, .name = MP_QSTR_function, #if MICROPY_CPYTHON_COMPAT .print = fun_bc_print, @@ -406,6 +412,7 @@ STATIC mp_obj_t fun_native_call(mp_obj_t self_in, size_t n_args, size_t n_kw, co STATIC const mp_obj_type_t mp_type_fun_native = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF, .name = MP_QSTR_function, .call = fun_native_call, .unary_op = mp_generic_unary_op, @@ -513,6 +520,7 @@ STATIC mp_obj_t fun_asm_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const STATIC const mp_obj_type_t mp_type_fun_asm = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF, .name = MP_QSTR_function, .call = fun_asm_call, .unary_op = mp_generic_unary_op, diff --git a/py/objgenerator.c b/py/objgenerator.c index 5e140ba23..543685fac 100644 --- a/py/objgenerator.c +++ b/py/objgenerator.c @@ -73,6 +73,7 @@ STATIC mp_obj_t gen_wrap_call(mp_obj_t self_in, size_t n_args, size_t n_kw, cons const mp_obj_type_t mp_type_gen_wrap = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF, .name = MP_QSTR_generator, .call = gen_wrap_call, .unary_op = mp_generic_unary_op, @@ -126,6 +127,7 @@ STATIC mp_obj_t native_gen_wrap_call(mp_obj_t self_in, size_t n_args, size_t n_k const mp_obj_type_t mp_type_native_gen_wrap = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF, .name = MP_QSTR_generator, .call = native_gen_wrap_call, .unary_op = mp_generic_unary_op, diff --git a/py/runtime.c b/py/runtime.c index 157bc74a8..78da38691 100644 --- a/py/runtime.c +++ b/py/runtime.c @@ -993,6 +993,7 @@ STATIC mp_obj_t checked_fun_call(mp_obj_t self_in, size_t n_args, size_t n_kw, c STATIC const mp_obj_type_t mp_type_checked_fun = { { &mp_type_type }, + .flags = MP_TYPE_FLAG_BINDS_SELF, .name = MP_QSTR_function, .call = checked_fun_call, }; @@ -1011,49 +1012,54 @@ STATIC mp_obj_t mp_obj_new_checked_fun(const mp_obj_type_t *type, mp_obj_t fun) // and put the result in the dest[] array for a possible method call. // Conversion means dealing with static/class methods, callables, and values. // see http://docs.python.org/3/howto/descriptor.html +// and also https://mail.python.org/pipermail/python-dev/2015-March/138950.html void mp_convert_member_lookup(mp_obj_t self, const mp_obj_type_t *type, mp_obj_t member, mp_obj_t *dest) { - if (mp_obj_is_type(member, &mp_type_staticmethod)) { - // return just the function - dest[0] = ((mp_obj_static_class_method_t *)MP_OBJ_TO_PTR(member))->fun; - } else if (mp_obj_is_type(member, &mp_type_classmethod)) { - // return a bound method, with self being the type of this object - // this type should be the type of the original instance, not the base - // type (which is what is passed in the 'type' argument to this function) - if (self != MP_OBJ_NULL) { - type = mp_obj_get_type(self); - } - dest[0] = ((mp_obj_static_class_method_t *)MP_OBJ_TO_PTR(member))->fun; - dest[1] = MP_OBJ_FROM_PTR(type); - } else if (mp_obj_is_type(member, &mp_type_type)) { - // Don't try to bind types (even though they're callable) - dest[0] = member; - } else if (mp_obj_is_fun(member) - || (mp_obj_is_obj(member) - && (((mp_obj_base_t *)MP_OBJ_TO_PTR(member))->type->name == MP_QSTR_closure - || ((mp_obj_base_t *)MP_OBJ_TO_PTR(member))->type->name == MP_QSTR_generator))) { - // only functions, closures and generators objects can be bound to self - #if MICROPY_BUILTIN_METHOD_CHECK_SELF_ARG + if (mp_obj_is_obj(member)) { const mp_obj_type_t *m_type = ((mp_obj_base_t *)MP_OBJ_TO_PTR(member))->type; - if (self == MP_OBJ_NULL - && (m_type == &mp_type_fun_builtin_0 - || m_type == &mp_type_fun_builtin_1 - || m_type == &mp_type_fun_builtin_2 - || m_type == &mp_type_fun_builtin_3 - || m_type == &mp_type_fun_builtin_var) - && type != &mp_type_object) { - // we extracted a builtin method without a first argument, so we must - // wrap this function in a type checker - // Note that object will do its own checking so shouldn't be wrapped. - dest[0] = mp_obj_new_checked_fun(type, member); - } else - #endif - { - // return a bound method, with self being this object + if (m_type->flags & MP_TYPE_FLAG_BINDS_SELF) { + // `member` is a function that binds self as its first argument. + if (m_type->flags & MP_TYPE_FLAG_BUILTIN_FUN) { + // `member` is a built-in function, which has special behaviour. + if (mp_obj_is_instance_type(type)) { + // Built-in functions on user types always behave like a staticmethod. + dest[0] = member; + } + #if MICROPY_BUILTIN_METHOD_CHECK_SELF_ARG + else if (self == MP_OBJ_NULL && type != &mp_type_object) { + // `member` is a built-in method without a first argument, so wrap + // it in a type checker that will check self when it's supplied. + // Note that object will do its own checking so shouldn't be wrapped. + dest[0] = mp_obj_new_checked_fun(type, member); + } + #endif + else { + // Return a (built-in) bound method, with self being this object. + dest[0] = member; + dest[1] = self; + } + } else { + // Return a bound method, with self being this object. + dest[0] = member; + dest[1] = self; + } + } else if (m_type == &mp_type_staticmethod) { + // `member` is a staticmethod, return the function that it wraps. + dest[0] = ((mp_obj_static_class_method_t *)MP_OBJ_TO_PTR(member))->fun; + } else if (m_type == &mp_type_classmethod) { + // `member` is a classmethod, return a bound method with self being the type of + // this object. This type should be the type of the original instance, not the + // base type (which is what is passed in the `type` argument to this function). + if (self != MP_OBJ_NULL) { + type = mp_obj_get_type(self); + } + dest[0] = ((mp_obj_static_class_method_t *)MP_OBJ_TO_PTR(member))->fun; + dest[1] = MP_OBJ_FROM_PTR(type); + } else { + // `member` is a value, so just return that value. dest[0] = member; - dest[1] = self; } } else { - // class member is a value, so just return that value + // `member` is a value, so just return that value. dest[0] = member; } } diff --git a/tests/basics/class_bind_self.py b/tests/basics/class_bind_self.py index 16e4f5ac9..813f87644 100644 --- a/tests/basics/class_bind_self.py +++ b/tests/basics/class_bind_self.py @@ -40,11 +40,18 @@ print(c.f2(2)) print(c.f3()) print(next(c.f4(4))) print(c.f5(5)) -#print(c.f6(-6)) not working in uPy +print(c.f6(-6)) print(c.f7(7)) print(c.f8(8)) print(c.f9(9)) +# test calling the functions accessed via the class itself +print(C.f5(10)) +print(C.f6(-11)) +print(C.f7(12)) +print(C.f8(13)) +print(C.f9(14)) + # not working in uPy #class C(list): # # this acts like a method and binds self