fix #688: optimised builtin functions/methods return 0 instead of None

Cython/Compiler/Builtin.py

@@ -478,9 +478,9 @@ builtin_types_table = [
 
     ("tuple",   "PyTuple_Type",    []),
 
-    ("list",    "PyList_Type",     [BuiltinMethod("insert",  "TzO",  "i", "PyList_Insert"),
-                                    BuiltinMethod("reverse", "T",    "i", "PyList_Reverse"),
-                                    BuiltinMethod("append",  "TO",   "i", "PyList_Append"),
+    ("list",    "PyList_Type",     [BuiltinMethod("insert",  "TzO",  "r", "PyList_Insert"),
+                                    BuiltinMethod("reverse", "T",    "r", "PyList_Reverse"),
+                                    BuiltinMethod("append",  "TO",   "r", "PyList_Append"),
                                     ]),
 
     ("dict",    "PyDict_Type",     [BuiltinMethod("items", "T",   "O", "PyDict_Items"),  # FIXME: Py3 mode?
@@ -494,9 +494,9 @@ builtin_types_table = [
                                     ]),
 #    ("file",    "PyFile_Type",     []),  # not in Py3
 
-    ("set",       "PySet_Type",    [BuiltinMethod("clear",   "T",  "i", "PySet_Clear"),
-                                    BuiltinMethod("discard", "TO", "i", "PySet_Discard"),
-                                    BuiltinMethod("add",     "TO", "i", "PySet_Add"),
+    ("set",       "PySet_Type",    [BuiltinMethod("clear",   "T",  "r", "PySet_Clear"),
+                                    BuiltinMethod("discard", "TO", "r", "PySet_Discard"),
+                                    BuiltinMethod("add",     "TO", "r", "PySet_Add"),
                                     BuiltinMethod("pop",     "T",  "O", "PySet_Pop")]),
     ("frozenset", "PyFrozenSet_Type", []),
 ]

Cython/Compiler/Optimize.py

@@ -1604,6 +1604,15 @@ class OptimizeBuiltinCalls(Visitor.EnvTransform):
             return node.operand
         return node
 
+    def visit_ExprStatNode(self, node):
+        """
+        Drop useless coercions.
+        """
+        self.visitchildren(node)
+        if isinstance(node.expr, ExprNodes.CoerceToPyTypeNode):
+            node.expr = node.expr.arg
+        return node
+
     def visit_CoerceToBooleanNode(self, node):
         """Drop redundant conversion nodes after tree changes.
         """
@@ -2146,7 +2155,7 @@ class OptimizeBuiltinCalls(Visitor.EnvTransform):
     _handle_simple_method_list_pop = _handle_simple_method_object_pop
 
     single_param_func_type = PyrexTypes.CFuncType(
-        PyrexTypes.c_int_type, [
+        PyrexTypes.c_returncode_type, [
             PyrexTypes.CFuncTypeArg("obj", PyrexTypes.py_object_type, None),
             ],
         exception_value = "-1")
@@ -2158,7 +2167,7 @@ class OptimizeBuiltinCalls(Visitor.EnvTransform):
             return node
         return self._substitute_method_call(
             node, "PyList_Sort", self.single_param_func_type,
-            'sort', is_unbound_method, args)
+            'sort', is_unbound_method, args).coerce_to(node.type, self.current_env)
 
     Pyx_PyDict_GetItem_func_type = PyrexTypes.CFuncType(
         PyrexTypes.py_object_type, [

Cython/Compiler/PyrexTypes.py

@@ -913,6 +913,8 @@ class CAnonEnumType(CIntType):
 
 class CReturnCodeType(CIntType):
 
+    to_py_function = "__Pyx_Owned_Py_None"
+
     is_returncode = 1
 
 
@@ -2783,6 +2785,7 @@ type_conversion_predeclarations = """
 #define __Pyx_PyBytes_FromUString(s) PyBytes_FromString((char*)s)
 #define __Pyx_PyBytes_AsUString(s)   ((unsigned char*) PyBytes_AsString(s))
 
+#define __Pyx_Owned_Py_None(b) (Py_INCREF(Py_None), Py_None)
 #define __Pyx_PyBool_FromLong(b) ((b) ? (Py_INCREF(Py_True), Py_True) : (Py_INCREF(Py_False), Py_False))
 static CYTHON_INLINE int __Pyx_PyObject_IsTrue(PyObject*);
 static CYTHON_INLINE PyObject* __Pyx_PyNumber_Int(PyObject* x);

tests/run/builtin_methods_return_values.pyx

+# mode: run
+# tag: list, set, builtins
+# ticket: 688
+
+_set = set
+
+class TestObj(object):
+    pass
+
+def _setattr(obj):
+    """
+    >>> t = TestObj()
+    >>> _setattr(t) is None
+    True
+    >>> t.test is None
+    True
+    """
+    setattr(obj, 'test', None)
+    return setattr(obj, 'test', None)
+
+def _delattr(obj):
+    """
+    >>> t = TestObj()
+    >>> t.test1 = t.test2 = True
+    >>> _delattr(t) is None
+    True
+    >>> hasattr(t, 'test1')
+    False
+    >>> hasattr(t, 'test2')
+    False
+    """
+    delattr(obj, 'test1')
+    return delattr(obj, 'test2')
+
+def list_sort(list l):
+    """
+    >>> list_sort([1,2,3]) is None
+    True
+    """
+    l.sort()
+    return l.sort()
+
+def list_reverse(list l):
+    """
+    >>> list_reverse([1,2,3]) is None
+    True
+    """
+    l.reverse()
+    return l.reverse()
+
+def list_insert(list l):
+    """
+    >>> list_insert([1,2,3]) is None
+    True
+    """
+    l.insert(1, 2)
+    return l.insert(1, 2)
+
+def list_append(list l):
+    """
+    >>> list_append([1,2,3]) is None
+    True
+    """
+    l.append(1)
+    return l.append(2)
+
+def set_clear(set s):
+    """
+    >>> set_clear(_set([1,2,3])) is None
+    True
+    """
+    s.clear()
+    return s.clear()
+
+def set_discard(set s):
+    """
+    >>> set_discard(_set([1,2,3])) is None
+    True
+    """
+    s.discard(1)
+    return s.discard(2)
+
+def set_add(set s):
+    """
+    >>> set_add(_set([1,2,3])) is None
+    True
+    """
+    s.add(1)
+    return s.add(2)