From 72698b0b970b4a5899229f458fcd9a7c4aa9a7cc Mon Sep 17 00:00:00 2001
From: Stefan Behnel <stefan_ml@behnel.de>
Date: Tue, 24 Dec 2013 14:54:09 +0100
Subject: [PATCH] optimise object.append() only when return value is not used
 and adapt signature accordingly

---
 Cython/Compiler/Optimize.py | 18 ++++++++++--------
 Cython/Utility/Optimize.c   | 14 ++++++++------
 tests/run/append.pyx        | 32 ++++++++++++++++++++++++++++----
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/Cython/Compiler/Optimize.py b/Cython/Compiler/Optimize.py
index d40ef3a..c66ba20 100644
--- a/Cython/Compiler/Optimize.py
+++ b/Cython/Compiler/Optimize.py
@@ -2357,25 +2357,27 @@ class OptimizeBuiltinCalls(Visitor.MethodDispatcherTransform):
     ### methods of builtin types
 
     PyObject_Append_func_type = PyrexTypes.CFuncType(
-        PyrexTypes.py_object_type, [
+        PyrexTypes.c_returncode_type, [
             PyrexTypes.CFuncTypeArg("list", PyrexTypes.py_object_type, None),
             PyrexTypes.CFuncTypeArg("item", PyrexTypes.py_object_type, None),
-            ])
+            ],
+        exception_value="-1")
 
     def _handle_simple_method_object_append(self, node, function, args, is_unbound_method):
         """Optimistic optimisation as X.append() is almost always
         referring to a list.
         """
-        if len(args) != 2:
+        if len(args) != 2 or node.result_is_used:
             return node
 
         return ExprNodes.PythonCapiCallNode(
             node.pos, "__Pyx_PyObject_Append", self.PyObject_Append_func_type,
-            args = args,
-            may_return_none = True,
-            is_temp = node.is_temp,
-            utility_code = load_c_utility('append')
-            )
+            args=args,
+            may_return_none=False,
+            is_temp=node.is_temp,
+            result_is_used=False,
+            utility_code=load_c_utility('append')
+        )
 
     PyObject_Pop_func_type = PyrexTypes.CFuncType(
         PyrexTypes.py_object_type, [
diff --git a/Cython/Utility/Optimize.c b/Cython/Utility/Optimize.c
index a5e1d57..4bd70df 100644
--- a/Cython/Utility/Optimize.c
+++ b/Cython/Utility/Optimize.c
@@ -8,20 +8,22 @@
 
 /////////////// append.proto ///////////////
 
-static CYTHON_INLINE PyObject* __Pyx_PyObject_Append(PyObject* L, PyObject* x); /*proto*/
+static CYTHON_INLINE int __Pyx_PyObject_Append(PyObject* L, PyObject* x); /*proto*/
 
 /////////////// append ///////////////
 //@requires: ListAppend
 //@requires: ObjectHandling.c::PyObjectCallMethod
 
-static CYTHON_INLINE PyObject* __Pyx_PyObject_Append(PyObject* L, PyObject* x) {
+static CYTHON_INLINE int __Pyx_PyObject_Append(PyObject* L, PyObject* x) {
     if (likely(PyList_CheckExact(L))) {
-        if (unlikely(__Pyx_PyList_Append(L, x) < 0)) return NULL;
-        Py_INCREF(Py_None);
-        return Py_None; /* this is just to have an accurate signature */
+        if (unlikely(__Pyx_PyList_Append(L, x) < 0)) return -1;
     } else {
-        return __Pyx_PyObject_CallMethod1(L, PYIDENT("append"), x);
+        PyObject* retval = __Pyx_PyObject_CallMethod1(L, PYIDENT("append"), x);
+        if (unlikely(!retval))
+            return -1;
+        Py_DECREF(retval);
     }
+    return 0;
 }
 
 /////////////// ListAppend.proto ///////////////
diff --git a/tests/run/append.pyx b/tests/run/append.pyx
index 9a8aff5..db6e865 100644
--- a/tests/run/append.pyx
+++ b/tests/run/append.pyx
@@ -1,6 +1,6 @@
 class A:
     def append(self, x):
-        print u"appending"
+        print u"appending", x
         return x
 
 class B(list):
@@ -8,6 +8,7 @@ class B(list):
         for arg in args:
             list.append(self, arg)
 
+
 def test_append(L):
     """
     >>> test_append([])
@@ -17,11 +18,11 @@ def test_append(L):
     got error
     [1, 2, (3, 4)]
     >>> _ = test_append(A())
-    appending
+    appending 1
     1
-    appending
+    appending 2
     2
-    appending
+    appending (3, 4)
     (3, 4)
     got error
     >>> test_append(B())
@@ -39,3 +40,26 @@ def test_append(L):
     except TypeError:
         print u"got error"
     return L
+
+
+def append_unused_retval(L):
+    """
+    >>> append_unused_retval([])
+    got error
+    [1, 2, (3, 4)]
+    >>> _ = append_unused_retval(A())
+    appending 1
+    appending 2
+    appending (3, 4)
+    got error
+    >>> append_unused_retval(B())
+    [1, 2, (3, 4), 5, 6]
+    """
+    L.append(1)
+    L.append(2)
+    L.append((3,4))
+    try:
+        L.append(5,6)
+    except TypeError:
+        print u"got error"
+    return L
-- 
2.7.4