fix ticket #653

- generate correct code for optimised override calls to methods of builtin types in subtypes
- optimise calls for 'final' subtypes
- disable optimised C-API calls for non-final subtypes

Cython/Compiler/Builtin.py

@@ -324,6 +324,7 @@ class BuiltinMethod(_BuiltinOverride):
             # override 'self' type (first argument)
             self_arg = PyrexTypes.CFuncTypeArg("", self_type, None)
             self_arg.not_none = True
+            self_arg.accept_builtin_subtypes = True
             method_type = sig.function_type(self_arg)
         self_type.scope.declare_builtin_cfunction(
             self.py_name, method_type, self.cname, utility_code = self.utility_code)

Cython/Compiler/ExprNodes.py

@@ -3087,8 +3087,11 @@ class SimpleCallNode(CallNode):
                         error = 'PyExc_AttributeError',
                         format_args = [self.function.entry.name])
                 expected_type = self_arg.type
-                self.coerced_self = CloneNode(self.self).coerce_to(
-                    expected_type, env)
+                if self_arg.accept_builtin_subtypes:
+                    self.coerced_self = CMethodSelfCloneNode(self.self)
+                else:
+                    self.coerced_self = CloneNode(self.self)
+                self.coerced_self = self.coerced_self.coerce_to(expected_type, env)
                 # Insert coerced 'self' argument into argument list.
                 self.args.insert(0, self.coerced_self)
             self.analyse_c_function_call(env)
@@ -3817,7 +3820,7 @@ class AttributeNode(ExprNode):
         obj_code = obj.result_as(obj.type)
         #print "...obj_code =", obj_code ###
         if self.entry and self.entry.is_cmethod:
-            if obj.type.is_extension_type:
+            if obj.type.is_extension_type and not self.entry.is_builtin_cmethod:
                 return "((struct %s *)%s%s%s)->%s" % (
                     obj.type.vtabstruct_cname, obj_code, self.op,
                     obj.type.vtabslot_cname, self.member)
@@ -7861,6 +7864,18 @@ class CloneNode(CoercionNode):
         pass
 
 
+class CMethodSelfCloneNode(CloneNode):
+    # Special CloneNode for the self argument of builtin C methods
+    # that accepts subtypes of the builtin type.  This is safe only
+    # for 'final' subtypes, as subtypes of the declared type may
+    # override the C method.
+
+    def coerce_to(self, dst_type, env):
+        if dst_type.is_builtin_type and self.type.subtype_of(dst_type):
+            return self
+        return CloneNode.coerce_to(self, dst_type, env)
+
+
 class ModuleRefNode(ExprNode):
     # Simple returns the module object
 

Cython/Compiler/Nodes.py

@@ -2229,7 +2229,7 @@ class DefNode(FuncDefNode):
         #print "DefNode.declare_pyfunction:", self.name, "in", env ###
         name = self.name
         entry = env.lookup_here(name)
-        if entry and entry.type.is_cfunction and not self.is_wrapper:
+        if entry and entry.type.is_cfunction and not entry.is_builtin_cmethod and not self.is_wrapper:
             warning(self.pos, "Overriding cdef method with def method.", 5)
         entry = env.declare_pyfunction(name, self.pos, allow_redefine=not self.is_wrapper)
         self.entry = entry

Cython/Compiler/PyrexTypes.py

@@ -537,7 +537,7 @@ class PyExtensionType(PyObjectType):
             scope.parent_type = self
 
     def subtype_of_resolved_type(self, other_type):
-        if other_type.is_extension_type:
+        if other_type.is_extension_type or other_type.is_builtin_type:
             return self is other_type or (
                 self.base_type and self.base_type.subtype_of(other_type))
         else:
@@ -1950,6 +1950,7 @@ class CFuncTypeArg(object):
     not_none = False
     or_none = False
     accept_none = True
+    accept_builtin_subtypes = False
 
     def __init__(self, name, type, pos, cname=None):
         self.name = name

Cython/Compiler/Symtab.py

@@ -68,6 +68,7 @@ class Entry(object):
     # is_variable      boolean    Is a variable
     # is_cfunction     boolean    Is a C function
     # is_cmethod       boolean    Is a C method of an extension type
+    # is_builtin_cmethod boolean  Is a C method of a builtin type (implies is_cmethod)
     # is_unbound_cmethod boolean  Is an unbound C method of an extension type
     # is_anonymous     boolean    Is a anonymous pyfunction entry
     # is_type          boolean    Is a type definition
@@ -136,6 +137,7 @@ class Entry(object):
     is_variable = 0
     is_cfunction = 0
     is_cmethod = 0
+    is_builtin_cmethod = False
     is_unbound_cmethod = 0
     is_anonymous = 0
     is_type = 0
@@ -1761,7 +1763,17 @@ class CClassScope(ClassScope):
     def lookup_here(self, name):
         if name == "__new__":
             name = EncodedString("__cinit__")
-        return ClassScope.lookup_here(self, name)
+        entry = ClassScope.lookup_here(self, name)
+        if entry and entry.is_builtin_cmethod:
+            if not self.parent_type.is_builtin_type:
+                # For subtypes of builtin types, we can only return
+                # optimised C methods if the type if final.
+                # Otherwise, subtypes may choose to override the
+                # method, but the optimisation would prevent the
+                # subtype method from being called.
+                if not self.directives['final']:
+                    return None
+        return entry
 
     def declare_cfunction(self, name, type, pos,
                           cname = None, visibility = 'private', api = 0, in_pxd = 0,
@@ -1843,17 +1855,24 @@ class CClassScope(ClassScope):
         def adapt(cname):
             return "%s.%s" % (Naming.obj_base_cname, base_entry.cname)
         for base_entry in \
-            base_scope.inherited_var_entries + base_scope.var_entries:
-                entry = self.declare(base_entry.name, adapt(base_entry.cname),
-                    base_entry.type, None, 'private')
-                entry.is_variable = 1
-                self.inherited_var_entries.append(entry)
+                base_scope.inherited_var_entries + base_scope.var_entries:
+            entry = self.declare(base_entry.name, adapt(base_entry.cname),
+                                 base_entry.type, None, 'private')
+            entry.is_variable = 1
+            self.inherited_var_entries.append(entry)
         for base_entry in base_scope.cfunc_entries:
+            cname = base_entry.cname
+            var_entry = base_entry.as_variable
+            is_builtin = var_entry and var_entry.is_builtin
+            if not is_builtin:
+                cname = adapt(cname)
             entry = self.add_cfunction(base_entry.name, base_entry.type,
-                                       base_entry.pos, adapt(base_entry.cname),
+                                       base_entry.pos, cname,
                                        base_entry.visibility, base_entry.func_modifiers)
             entry.is_inherited = 1
-
+            if is_builtin:
+                entry.is_builtin_cmethod = True
+                entry.as_variable = var_entry
 
 class CppClassScope(Scope):
     #  Namespace of a C++ class.

tests/run/builtin_subtype_methods_T653.pyx

@@ -2,7 +2,12 @@
 # mode: run
 # ticket: 653
 
+cimport cython
+
 cdef class MyDict(dict):
+    @cython.test_assert_path_exists("//ComprehensionNode//AttributeNode",
+                                    "//ComprehensionNode//AttributeNode[@attribute='items']")
+    @cython.test_fail_if_path_exists("//ComprehensionNode//CMethodSelfCloneNode")
     def test_items(self):
         """
         >>> MyDict(a=1, b=2).test_items()
@@ -20,3 +25,119 @@ cdef class MyDict(dict):
         l = [ (key, value) for key, value in self.iteritems() ]
         l.sort()
         return l
+
+@cython.final
+cdef class MyDictFinal(dict):
+    @cython.test_assert_path_exists("//ComprehensionNode//CMethodSelfCloneNode")
+    def test_items(self):
+        """
+        >>> MyDictFinal(a=1, b=2).test_items()
+        [('a', 1), ('b', 2)]
+        """
+        l = [ (key, value) for key, value in self.items() ]
+        l.sort()
+        return l
+
+    def test_iteritems(self):
+        """
+        >>> MyDictFinal(a=1, b=2).test_iteritems()
+        [('a', 1), ('b', 2)]
+        """
+        l = [ (key, value) for key, value in self.iteritems() ]
+        l.sort()
+        return l
+
+cdef class MyDict2(MyDict):
+    @cython.test_assert_path_exists("//ComprehensionNode//AttributeNode",
+                                    "//ComprehensionNode//AttributeNode[@attribute='items']")
+    @cython.test_fail_if_path_exists("//ComprehensionNode//CMethodSelfCloneNode")
+    def test_items(self):
+        """
+        >>> MyDict2(a=1, b=2).test_items()
+        [('a', 1), ('b', 2)]
+        """
+        l = [ (key, value) for key, value in self.items() ]
+        l.sort()
+        return l
+
+    def test_iteritems(self):
+        """
+        >>> MyDict2(a=1, b=2).test_iteritems()
+        [('a', 1), ('b', 2)]
+        """
+        l = [ (key, value) for key, value in self.iteritems() ]
+        l.sort()
+        return l
+
+@cython.final
+cdef class MyDict2Final(MyDict):
+    @cython.test_assert_path_exists("//ComprehensionNode//CMethodSelfCloneNode")
+    def test_items(self):
+        """
+        >>> MyDict2Final(a=1, b=2).test_items()
+        [('a', 1), ('b', 2)]
+        """
+        l = [ (key, value) for key, value in self.items() ]
+        l.sort()
+        return l
+
+    def test_iteritems(self):
+        """
+        >>> MyDict2Final(a=1, b=2).test_iteritems()
+        [('a', 1), ('b', 2)]
+        """
+        l = [ (key, value) for key, value in self.iteritems() ]
+        l.sort()
+        return l
+
+@cython.final
+cdef class MyDictOverride(dict):
+    def items(self):
+        return [(1,2), (3,4)]
+
+    @cython.test_assert_path_exists("//ComprehensionNode//AttributeNode",
+                                    "//ComprehensionNode//AttributeNode[@attribute='items']")
+    @cython.test_fail_if_path_exists("//ComprehensionNode//CMethodSelfCloneNode")
+    def test_items(self):
+        """
+        >>> MyDictOverride(a=1, b=2).test_items()
+        [(1, 2), (3, 4)]
+        """
+        l = [ (key, value) for key, value in self.items() ]
+        l.sort()
+        return l
+
+    def test_iteritems(self):
+        """
+        >>> MyDictOverride(a=1, b=2).test_iteritems()
+        [('a', 1), ('b', 2)]
+        """
+        l = [ (key, value) for key, value in self.iteritems() ]
+        l.sort()
+        return l
+
+@cython.final
+cdef class MyDictOverride2(MyDict):
+    def items(self):
+        return [(1,2), (3,4)]
+
+    @cython.test_assert_path_exists("//ComprehensionNode//AttributeNode",
+                                    "//ComprehensionNode//AttributeNode[@attribute='items']")
+    @cython.test_fail_if_path_exists("//ComprehensionNode//CMethodSelfCloneNode")
+    def test_items(self):
+        """
+        >>> MyDictOverride2(a=1, b=2).test_items()
+        [(1, 2), (3, 4)]
+        """
+        l = [ (key, value) for key, value in self.items() ]
+        l.sort()
+        return l
+
+    def test_iteritems(self):
+        """
+        >>> MyDictOverride2(a=1, b=2).test_iteritems()
+        [('a', 1), ('b', 2)]
+        """
+        l = [ (key, value) for key, value in self.iteritems() ]
+        l.sort()
+        return l