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 <damien@micropython.org>

5 files changed, 59 insertions, 38 deletions

diff --git a/py/obj.h b/py/obj.h
index 6bec57c352..f621b1dadd 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 f5038ffc46..9a8c7631c2 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 f9a6b4ebc2..052f4b1ced 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 5e140ba234..543685fac7 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 157bc74a8d..78da386919 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;
     }
 }