Optimise list.extend([a,b,c]) into fast sequential calls to list.append().

CHANGES.rst

@@ -36,6 +36,9 @@ Features added
 * Iteration over sets and frozensets is optimised.
   (Github issue #2048)
 
+* ``alist.extend([a,b,c])`` is optimised into sequential ``list.append()`` calls
+  for short literal sequences.
+
 * Calls to builtin methods that are not specifically optimised into C-API calls
   now use a cache that avoids repeated lookups of the underlying C function.
   (Github issue #2054)

Cython/Compiler/Nodes.py

@@ -5055,7 +5055,7 @@ class ExprStatNode(StatNode):
         self.expr.result_is_used = False  # hint that .result() may safely be left empty
         self.expr.generate_evaluation_code(code)
         if not self.expr.is_temp and self.expr.result():
-            code.putln("%s;" % self.expr.result())
+            code.putln("(void)(%s);" % self.expr.result())
         self.expr.generate_disposal_code(code)
         self.expr.free_temps(code)
 

Cython/Compiler/Optimize.py

@@ -2046,11 +2046,18 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
 
     def visit_ExprStatNode(self, node):
         """
-        Drop useless coercions.
+        Drop dead code and useless coercions.
         """
         self.visitchildren(node)
         if isinstance(node.expr, ExprNodes.CoerceToPyTypeNode):
             node.expr = node.expr.arg
+        expr = node.expr
+        if expr is None or expr.is_none or expr.is_literal:
+            # Expression was removed or is dead code => remove ExprStatNode as well.
+            return None
+        if expr.is_name and expr.entry and (expr.entry.is_local or expr.entry.is_arg):
+            # Ignore dead references to local variables etc.
+            return None
         return node
 
     def visit_CoerceToBooleanNode(self, node):
@@ -2815,6 +2822,62 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
             utility_code=load_c_utility('append')
         )
 
+    def _handle_simple_method_list_extend(self, node, function, args, is_unbound_method):
+        """Replace list.extend([...]) for short sequence literals values by sequential appends
+        to avoid creating an intermediate sequence argument.
+        """
+        if len(args) != 2:
+            return node
+        obj, value = args
+        if not value.is_sequence_constructor or value.mult_factor is not None:
+            return node
+        items = list(value.args)
+        if len(items) > 4:
+            # Appending wins for short sequences.
+            # Ignorantly assume that this a good enough limit that avoids repeated resizing.
+            return node
+        wrapped_obj = self._wrap_self_arg(obj, function, is_unbound_method, 'extend')
+        if not items:
+            # Empty sequences are not likely to occur, but why waste a call to list.extend() for them?
+            wrapped_obj.result_is_used = node.result_is_used
+            return wrapped_obj
+        cloned_obj = obj = wrapped_obj
+        if len(items) > 1 and not obj.is_simple():
+            cloned_obj = UtilNodes.LetRefNode(obj)
+        # Use ListComp_Append() for all but the last item and finish with PyList_Append()
+        # to shrink the list storage size at the very end if necessary.
+        temps = []
+        arg = items[-1]
+        if not arg.is_simple():
+            arg = UtilNodes.LetRefNode(arg)
+            temps.append(arg)
+        new_node = ExprNodes.PythonCapiCallNode(
+            node.pos, "__Pyx_PyList_Append", self.PyObject_Append_func_type,
+            args=[cloned_obj, arg],
+            is_temp=True,
+            utility_code=load_c_utility("ListAppend"))
+        for arg in items[-2::-1]:
+            if not arg.is_simple():
+                arg = UtilNodes.LetRefNode(arg)
+                temps.append(arg)
+            new_node = ExprNodes.binop_node(
+                node.pos, '|',
+                ExprNodes.PythonCapiCallNode(
+                    node.pos, "__Pyx_ListComp_Append", self.PyObject_Append_func_type,
+                    args=[cloned_obj, arg], py_name="extend",
+                    is_temp=True,
+                    utility_code=load_c_utility("ListCompAppend")),
+                new_node,
+                type=PyrexTypes.c_returncode_type,
+            )
+        new_node.result_is_used = node.result_is_used
+        if cloned_obj is not obj:
+            temps.append(cloned_obj)
+        for temp in temps:
+            new_node = UtilNodes.EvalWithTempExprNode(temp, new_node)
+            new_node.result_is_used = node.result_is_used
+        return new_node
+
     PyByteArray_Append_func_type = PyrexTypes.CFuncType(
         PyrexTypes.c_returncode_type, [
             PyrexTypes.CFuncTypeArg("bytearray", PyrexTypes.py_object_type, None),
@@ -3791,18 +3854,8 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
                                 may_return_none=ExprNodes.PythonCapiCallNode.may_return_none,
                                 with_none_check=True):
         args = list(args)
-        if with_none_check and args and not args[0].is_literal:
-            self_arg = args[0]
-            if is_unbound_method:
-                self_arg = self_arg.as_none_safe_node(
-                    "descriptor '%s' requires a '%s' object but received a 'NoneType'",
-                    format_args=[attr_name, function.obj.name])
-            else:
-                self_arg = self_arg.as_none_safe_node(
-                    "'NoneType' object has no attribute '%{0}s'".format('.30' if len(attr_name) <= 30 else ''),
-                    error = "PyExc_AttributeError",
-                    format_args = [attr_name])
-            args[0] = self_arg
+        if with_none_check and args:
+            args[0] = self._wrap_self_arg(args[0], function, is_unbound_method, attr_name)
         if is_temp is None:
             is_temp = node.is_temp
         return ExprNodes.PythonCapiCallNode(
@@ -3814,6 +3867,20 @@ class OptimizeBuiltinCalls(Visitor.NodeRefCleanupMixin,
             result_is_used = node.result_is_used,
             )
 
+    def _wrap_self_arg(self, self_arg, function, is_unbound_method, attr_name):
+        if self_arg.is_literal:
+            return self_arg
+        if is_unbound_method:
+            self_arg = self_arg.as_none_safe_node(
+                "descriptor '%s' requires a '%s' object but received a 'NoneType'",
+                format_args=[attr_name, self_arg.type.name])
+        else:
+            self_arg = self_arg.as_none_safe_node(
+                "'NoneType' object has no attribute '%{0}s'".format('.30' if len(attr_name) <= 30 else ''),
+                error="PyExc_AttributeError",
+                format_args=[attr_name])
+        return self_arg
+
     def _inject_int_default_argument(self, node, args, arg_index, type, default_value):
         assert len(args) >= arg_index
         if len(args) == arg_index:

Cython/Compiler/Visitor.py

@@ -581,15 +581,23 @@ class MethodDispatcherTransform(EnvTransform):
             # into a C function call (defined in the builtin scope)
             if not function.entry:
                 return node
+            entry = function.entry
             is_builtin = (
-                function.entry.is_builtin or
-                function.entry is self.current_env().builtin_scope().lookup_here(function.name))
+                entry.is_builtin or
+                entry is self.current_env().builtin_scope().lookup_here(function.name))
             if not is_builtin:
                 if function.cf_state and function.cf_state.is_single:
                     # we know the value of the variable
                     # => see if it's usable instead
                     return self._delegate_to_assigned_value(
                         node, function, arg_list, kwargs)
+                if arg_list and entry.is_cmethod and entry.scope and entry.scope.parent_type.is_builtin_type:
+                    if entry.scope.parent_type is arg_list[0].type:
+                        # Optimised (unbound) method of a builtin type => try to "de-optimise".
+                        return self._dispatch_to_method_handler(
+                            entry.name, self_arg=None, is_unbound_method=True,
+                            type_name=entry.scope.parent_type.name,
+                            node=node, function=function, arg_list=arg_list, kwargs=kwargs)
                 return node
             function_handler = self._find_handler(
                 "function_%s" % function.name, kwargs)
@@ -615,8 +623,7 @@ class MethodDispatcherTransform(EnvTransform):
             obj_type = self_arg.type
             is_unbound_method = False
             if obj_type.is_builtin_type:
-                if (obj_type is Builtin.type_type and self_arg.is_name and
-                        arg_list and arg_list[0].type.is_pyobject):
+                if obj_type is Builtin.type_type and self_arg.is_name and arg_list and arg_list[0].type.is_pyobject:
                     # calling an unbound method like 'list.append(L,x)'
                     # (ignoring 'type.mro()' here ...)
                     type_name = self_arg.name

tests/run/list.pyx

@@ -79,6 +79,10 @@ def test_list_reverse():
     l1.reverse()
     return l1
 
+
+@cython.test_assert_path_exists(
+    '//SimpleCallNode//AttributeNode[@entry.cname = "__Pyx_PyList_Append"]',
+)
 def test_list_append():
     """
     >>> test_list_append()
@@ -89,6 +93,36 @@ def test_list_append():
     l1.append(4)
     return l1
 
+
+@cython.test_assert_path_exists(
+    '//SimpleCallNode//NameNode[@entry.cname = "__Pyx_PyList_Append"]',
+)
+def test_list_append_unbound():
+    """
+    >>> test_list_append_unbound()
+    [1, 2, 3, 4]
+    """
+    cdef list l1 = [1,2]
+    list.append(l1, 3)
+    list.append(l1, 4)
+    return l1
+
+
+@cython.test_assert_path_exists(
+    '//SimpleCallNode//NameNode[@entry.cname = "__Pyx_PyList_Append"]',
+)
+def test_list_append_unbound_assigned():
+    """
+    >>> test_list_append_unbound_assigned()
+    [1, 2, 3, 4]
+    """
+    append = list.append
+    cdef list l1 = [1,2]
+    append(l1, 3)
+    append(l1, 4)
+    return l1
+
+
 def test_list_append_insert():
     """
     >>> test_list_append_insert()
@@ -138,20 +172,127 @@ def test_list_pop_all():
         return i == 2
     return False
 
-def test_list_extend():
+
+@cython.test_assert_path_exists(
+    '//PythonCapiCallNode//PythonCapiFunctionNode[@cname = "__Pyx_ListComp_Append"]',
+    '//PythonCapiCallNode//PythonCapiFunctionNode[@cname = "__Pyx_PyList_Append"]',
+)
+def test_list_extend(seq=None, x=4):
     """
     >>> test_list_extend()
-    [1, 2, 3, 4, 5, 6]
+    [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+    >>> test_list_extend([])
+    [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+    >>> test_list_extend([1])
+    [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 1]
+    >>> test_list_extend([1, 2])
+    [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 1, 2]
     """
     cdef list l = [1,2,3]
     l.extend([])
     l.extend(())
-    l.extend(set())
+    l.extend(set())  # not currently optimised (not worth the trouble)
     assert l == [1,2,3]
     assert len(l) == 3
-    l.extend([4,5,6])
+    l.extend([4,x+1,6])
+    l.extend([7,8,9,10,11,12,13,14])
+    if seq is not None:
+        l.extend(seq)
     return l
 
+
+@cython.test_assert_path_exists(
+    '//PythonCapiCallNode//PythonCapiFunctionNode[@cname = "__Pyx_ListComp_Append"]',
+    '//PythonCapiCallNode//PythonCapiFunctionNode[@cname = "__Pyx_PyList_Append"]',
+)
+def test_list_extend_unbound(seq=None, x=4):
+    """
+    >>> test_list_extend_unbound()
+    [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+    >>> test_list_extend_unbound([])
+    [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+    >>> test_list_extend_unbound([1])
+    [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 1]
+    >>> test_list_extend_unbound([1, 2])
+    [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 1, 2]
+    """
+    cdef list l = [1,2,3]
+    list.extend(l, [])
+    list.extend(l, ())
+    try:
+        list.extend((), ())
+    except TypeError:
+        pass
+    else:
+        assert False, "TypeError not raised!"
+    list.extend(l, set())  # not currently optimised (not worth the trouble)
+    assert l == [1,2,3]
+    assert len(l) == 3
+    list.extend(l, [4,x+1,6])
+    list.extend(l, [7,8,9,10,11,12,13,14])
+    if seq is not None:
+        list.extend(l, seq)
+    return l
+
+@cython.test_assert_path_exists(
+    '//PythonCapiCallNode//PythonCapiFunctionNode[@cname = "__Pyx_ListComp_Append"]',
+    '//PythonCapiCallNode//PythonCapiFunctionNode[@cname = "__Pyx_PyList_Append"]',
+)
+def test_list_extend_sideeffect(seq=None, exc=False):
+    """
+    >>> test_list_extend_sideeffect()
+    ([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16], [4, 6, 7, 8])
+    >>> test_list_extend_sideeffect([])
+    ([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16], [4, 6, 7, 8])
+    >>> test_list_extend_sideeffect([], exc=True)
+    ([1, 2, 3, 10, 11, 12, 13, 14, 15, 16], [4, 7, 8])
+    >>> test_list_extend_sideeffect([1])
+    ([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1], [4, 6, 7, 8])
+    >>> test_list_extend_sideeffect([1], exc=True)
+    ([1, 2, 3, 10, 11, 12, 13, 14, 15, 16, 1], [4, 7, 8])
+    >>> test_list_extend_sideeffect([1, 2])
+    ([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 1, 2], [4, 6, 7, 8])
+    """
+    calls = []
+    def sideeffect(value):
+        calls.append(value)
+        return value
+    def fail(value):
+        if exc:
+            raise TypeError("HUHU")
+        return value
+
+    cdef list l = [1,2,3]
+    l.extend([])
+    l.extend(())
+    l.extend(set())  # not currently optimised (not worth the trouble)
+    assert l == [1,2,3]
+    assert len(l) == 3
+
+    # Must first build all items, then append them in order.
+    # If building one value fails, none of them must be appended.
+    try:
+        l.extend([sideeffect(4), fail(5), sideeffect(6)])
+    except TypeError as e:
+        assert exc
+        assert "HUHU" in str(e)
+    else:
+        assert not exc
+
+    try:
+        l.extend([sideeffect(7), sideeffect(8), fail(9)])
+    except TypeError as e:
+        assert exc
+        assert "HUHU" in str(e)
+    else:
+        assert not exc
+
+    l.extend([10,11,12,13,14,15,16])
+    if seq is not None:
+        l.extend(seq)
+    return l, calls
+
+
 def test_none_list_extend(list l):
     """
     >>> test_none_list_extend([])

tests/run/tuple.pyx

@@ -139,3 +139,29 @@ def tuple_of_tuple_or_none(tuple x):
     TypeError: ...itera...
     """
     return tuple(tuple(tuple(x)))
+
+
+@cython.test_fail_if_path_exists(
+    "//ExprStatNode//TupleNode",
+    "//ExprStatNode",
+)
+def unused_literals():
+    """
+    >>> unused_literals()
+    """
+    (1, 2, 3)
+    (1, 2, 3 + 4)
+    ("abc", 'def')
+    #(int(), 2, 3)
+
+
+@cython.test_assert_path_exists(
+    "//ExprStatNode",
+    "//ExprStatNode//TupleNode",
+)
+def unused_non_literal():
+    """
+    >>> unused_non_literal()
+    """
+    (set(), None)
+    (range(10), None)