Merge pull request #353 from BigFav/reverse_range_optimize

Optimize reversed(range(...)), with non-constant values.

Cython/Compiler/Optimize.py

@@ -657,26 +657,86 @@ class IterationTransform(Visitor.EnvTransform):
 
         relation1, relation2 = self._find_for_from_node_relations(step_value < 0, reversed)
 
+        bound2_ref_node = None
         if reversed:
             bound1, bound2 = bound2, bound1
             abs_step = abs(step_value)
             if abs_step != 1:
-                if not (isinstance(bound1.constant_result, (int, long)) and
+                if (isinstance(bound1.constant_result, (int, long)) and
                         isinstance(bound2.constant_result, (int, long))):
-                    # FIXME: calculate final bounds at runtime
-                    return node
-                if step_value < 0:
-                    begin_value = bound2.constant_result
-                    end_value = bound1.constant_result
-                    bound1_value = begin_value - abs_step * ((begin_value - end_value - 1) // abs_step) - 1
+                    if step_value < 0:
+                        begin_value = bound2.constant_result
+                        end_value = bound1.constant_result
+                        bound1_value = begin_value - abs_step * ((begin_value - end_value - 1) // abs_step) - 1
+                    else:
+                        begin_value = bound1.constant_result
+                        end_value = bound2.constant_result
+                        bound1_value = end_value + abs_step * ((begin_value - end_value - 1) // abs_step) + 1
+
+                    bound1 = ExprNodes.IntNode(
+                        bound1.pos, value=str(bound1_value), constant_result=bound1_value,
+                        type=PyrexTypes.spanning_type(bound1.type, bound2.type))
                 else:
-                    begin_value = bound1.constant_result
-                    end_value = bound2.constant_result
-                    bound1_value = end_value + abs_step * ((begin_value - end_value - 1) // abs_step) + 1
-
-                bound1 = ExprNodes.IntNode(
-                    bound1.pos, value=str(bound1_value), constant_result=bound1_value,
-                    type=PyrexTypes.spanning_type(bound1.type, bound2.type))
+                    bound2_ref_node = UtilNodes.LetRefNode(bound2)
+                    spanning_type = PyrexTypes.spanning_type(bound1.type, bound2.type)
+                    spanning_step_type = PyrexTypes.spanning_type(spanning_type, step.type)
+
+                    if step_value < 0:
+                        begin_value = bound2_ref_node
+                        end_value = bound1
+                        final_op = '-'
+                        final_node = ExprNodes.SubNode
+                    else:
+                        begin_value = bound1
+                        end_value = bound2_ref_node
+                        final_op = '+'
+                        final_node = ExprNodes.AddNode
+
+                    bound1 = final_node(
+                        bound1.pos,
+                        operand1=final_node(
+                            bound1.pos,
+                            operand1=bound2_ref_node,
+                            operator=final_op,
+                            operand2=ExprNodes.MulNode(
+                                bound1.pos,
+                                operand1=ExprNodes.IntNode(
+                                    bound1.pos,
+                                    value=str(abs_step),
+                                    constant_value=abs_step,
+                                    type=spanning_step_type),
+                                operator='*',
+                                operand2=ExprNodes.DivNode(
+                                    bound1.pos, 
+                                    operand1=ExprNodes.SubNode(
+                                        bound1.pos,
+                                        operand1=ExprNodes.SubNode(
+                                            bound1.pos,
+                                            operand1=begin_value,
+                                            operator='-',
+                                            operand2=end_value,
+                                            type=spanning_type),
+                                        operator='-',
+                                        operand2=ExprNodes.IntNode(
+                                            bound1.pos,
+                                            value='1',
+                                            constant_result=1),
+                                        type=spanning_step_type),
+                                    operator='//',
+                                    operand2=ExprNodes.IntNode(
+                                        bound1.pos,
+                                        value=str(abs_step),
+                                        constant_value=abs_step,
+                                        type=spanning_step_type),
+                                    type=spanning_step_type),
+                                type=spanning_step_type),
+                            type=spanning_step_type),
+                        operator=final_op,
+                        operand2=ExprNodes.IntNode(
+                            bound1.pos,
+                            value='1',
+                            constant_result=1),
+                        type=spanning_type)
 
         if step_value < 0:
             step_value = -step_value
@@ -687,7 +747,10 @@ class IterationTransform(Visitor.EnvTransform):
         if not bound2.is_literal:
             # stop bound must be immutable => keep it in a temp var
             bound2_is_temp = True
-            bound2 = UtilNodes.LetRefNode(bound2)
+            if bound2_ref_node:
+                bound2 = bound2_ref_node
+            else:
+                bound2 = UtilNodes.LetRefNode(bound2)
         else:
             bound2_is_temp = False
 

tests/errors/reversed_literal_pyobjs.pyx

+# mode: error
+# tag: reversed
+
+cdef int i, j
+for i in reversed(range([], j, 2)):
+    pass
+for i in reversed(range([], j, -2)):
+    pass
+for i in reversed(range(j, [], 2)):
+    pass
+for i in reversed(range(j, [], -2)):
+    pass
+for i in reversed(range({}, j, 2)):
+    pass
+for i in reversed(range({}, j, -2)):
+    pass
+for i in reversed(range(j, {}, 2)):
+    pass
+for i in reversed(range(j, {}, -2)):
+    pass
+
+_ERRORS = """
+5:24: Cannot coerce list to type 'long'
+7:24: Cannot coerce list to type 'long'
+9:27: Cannot coerce list to type 'long'
+11:27: Cannot coerce list to type 'long'
+13:24: Cannot interpret dict as type 'long'
+15:24: Cannot interpret dict as type 'long'
+17:27: Cannot interpret dict as type 'long'
+19:27: Cannot interpret dict as type 'long'
+"""

tests/run/reversed_iteration.pyx

@@ -133,8 +133,8 @@ def reversed_range_step_neg(int a, int b):
         result.append(i)
     return result, i
 
-
-#cython.test_assert_path_exists('//ForFromStatNode')
+@cython.test_assert_path_exists('//ForFromStatNode')
+@cython.test_fail_if_path_exists('//ForInStatNode')
 def reversed_range_step3(int a, int b):
     """
     >>> [ i for i in _reversed(range(-5, 0, 3)) ]
@@ -151,6 +151,11 @@ def reversed_range_step3(int a, int b):
     []
     >>> reversed_range_step3(5, 0)
     ([], 99)
+
+    >>> [ i for i in _reversed(range(1, 1, 3)) ]
+    []
+    >>> reversed_range_step3(1, 1)
+    ([], 99)
     """
     cdef int i = 99
     result = []
@@ -158,8 +163,182 @@ def reversed_range_step3(int a, int b):
         result.append(i)
     return result, i
 
+@cython.test_assert_path_exists('//ForFromStatNode')
+@cython.test_fail_if_path_exists('//ForInStatNode')
+def reversed_range_step3_expr(int a, int b):
+    """
+    >>> [ i for i in _reversed(range(0, 5, 3)) ]
+    [3, 0]
+    >>> reversed_range_step3_expr(0, 5)
+    ([3, 0], 0)
+    """
+    cdef int i = 99, c = 100
+    result = []
+    for i in reversed(range(c-c + a + c-c, c-c + b + c-c, 3)):
+        result.append(i)
+    return result, i
 
 @cython.test_assert_path_exists('//ForFromStatNode')
+@cython.test_fail_if_path_exists('//ForInStatNode')
+def reversed_range_step3_neg(int a, int b):
+    """
+    >>> [ i for i in _reversed(range(0, -5, -3)) ]
+    [-3, 0]
+    >>> reversed_range_step3_neg(0, -5)
+    ([-3, 0], 0)
+
+    >>> [ i for i in _reversed(range(5, 0, -3)) ]
+    [2, 5]
+    >>> reversed_range_step3_neg(5, 0)
+    ([2, 5], 5)
+
+    >>> [ i for i in _reversed(range(0, 5, -3)) ]
+    []
+    >>> reversed_range_step3_neg(0, 5)
+    ([], 99)
+
+    >>> [ i for i in _reversed(range(1, 1, -3)) ]
+    []
+    >>> reversed_range_step3_neg(1, 1)
+    ([], 99)
+    """
+    cdef int i = 99
+    result = []
+    for i in reversed(range(a, b, -3)):
+        result.append(i)
+    return result, i
+
+@cython.test_assert_path_exists('//ForFromStatNode')
+@cython.test_fail_if_path_exists('//ForInStatNode')
+def reversed_range_step3_neg_expr(int a, int b):
+    """
+    >>> [ i for i in _reversed(range(5, 0, -3)) ]
+    [2, 5]
+    >>> reversed_range_step3_neg_expr(5, 0)
+    ([2, 5], 5)
+    """
+    cdef int i = 99, c = 100
+    result = []
+    for i in reversed(range(c-c + a + c-c, c-c + b + c-c, -3)):
+        result.append(i)
+    return result, i
+
+def reversed_range_step3_py_args(a, b):
+    """
+    >>> [ i for i in _reversed(range(-5, 0, 3)) ]
+    [-2, -5]
+    >>> reversed_range_step3_py_args(-5, 0)
+    ([-2, -5], -5)
+
+    >>> [ i for i in _reversed(range(0, 5, 3)) ]
+    [3, 0]
+    >>> reversed_range_step3_py_args(0, 5)
+    ([3, 0], 0)
+
+    >>> [ i for i in _reversed(range(5, 0, 3)) ]
+    []
+    >>> reversed_range_step3_py_args(5, 0)
+    ([], 99)
+
+    >>> [ i for i in _reversed(range(1, 1, 3)) ]
+    []
+    >>> reversed_range_step3_py_args(1, 1)
+    ([], 99)
+
+    >>> reversed_range_step3_py_args(set(), 1) # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    TypeError: ...integer...
+
+    >>> reversed_range_step3_py_args(1, set()) # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    TypeError: ...integer...
+    """
+    i = 99
+    result = []
+    for i in reversed(range(a, b, 3)):
+        result.append(i)
+    return result, i
+
+def reversed_range_step3_neg_py_args(a, b):
+    """
+    >>> [ i for i in _reversed(range(0, -5, -3)) ]
+    [-3, 0]
+    >>> reversed_range_step3_neg_py_args(0, -5)
+    ([-3, 0], 0)
+
+    >>> [ i for i in _reversed(range(5, 0, -3)) ]
+    [2, 5]
+    >>> reversed_range_step3_neg_py_args(5, 0)
+    ([2, 5], 5)
+
+    >>> [ i for i in _reversed(range(0, 5, -3)) ]
+    []
+    >>> reversed_range_step3_neg_py_args(0, 5)
+    ([], 99)
+
+    >>> [ i for i in _reversed(range(1, 1, -3)) ]
+    []
+    >>> reversed_range_step3_neg_py_args(1, 1)
+    ([], 99)
+
+    >>> reversed_range_step3_neg_py_args(set(), 1) # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    TypeError: ...integer...
+
+    >>> reversed_range_step3_neg_py_args(1, set()) # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    TypeError: ...integer...
+    """
+    i = 99
+    result = []
+    for i in reversed(range(a, b, -3)):
+        result.append(i)
+    return result, i
+
+def reversed_range_step3_py_obj_left(a, int b):
+    """
+    >>> reversed_range_step3_py_obj_left(set(), 0)
+    Traceback (most recent call last):
+    TypeError: an integer is required
+    """
+    cdef int i
+    result = []
+    for i in reversed(range(a, b, 3)):
+        result.append(i)
+
+def reversed_range_step3_py_obj_right(int a, b):
+    """
+    >>> reversed_range_step3_py_obj_right(0, set())
+    Traceback (most recent call last):
+    TypeError: an integer is required
+    """
+    cdef int i
+    result = []
+    for i in reversed(range(a, b, 3)):
+        result.append(i)
+
+def reversed_range_step3_neg_py_obj_left(a, int b):
+    """
+    >>> reversed_range_step3_neg_py_obj_left(set(), 0)
+    Traceback (most recent call last):
+    TypeError: an integer is required
+    """
+    cdef int i
+    result = []
+    for i in reversed(range(a, b, -3)):
+        result.append(i)
+
+def reversed_range_step3_neg_py_obj_right(int a, b):
+    """
+    >>> reversed_range_step3_py_obj_right(0, set())
+    Traceback (most recent call last):
+    TypeError: an integer is required
+    """
+    cdef int i
+    result = []
+    for i in reversed(range(a, b, -3)):
+        result.append(i)
+
 @cython.test_fail_if_path_exists('//ForInStatNode')
 def reversed_range_constant():
     """
@@ -175,7 +354,6 @@ def reversed_range_constant():
     assert result == list(reversed(range(1, 1, 4))), result
     assert i == 99
 
-    result = []
     for i in reversed(range(1, 1, 1)):
         result.append(i)
     assert result == list(reversed(range(1, 1, 1))), result
@@ -245,13 +423,13 @@ def reversed_range_constant():
         result.append(i)
     assert result == list(reversed(range(0, 12, 4))), result
 
+    i = 99
     result = []
     for i in reversed(range(-12, -2, 4)):
         result.append(i)
     assert result == list(reversed(range(-12, -2, 4))), result
     return result, i
 
-
 @cython.test_assert_path_exists('//ForFromStatNode')
 @cython.test_fail_if_path_exists('//ForInStatNode')
 def reversed_range_constant_neg():
@@ -387,7 +565,6 @@ def reversed_range_constant_neg():
         result.append(i)
     assert result == list(reversed(range(-2, -12, -1))), result
 
-
 unicode_string = u"abcDEF"
 
 @cython.test_assert_path_exists('//ForFromStatNode')