Opened 6 years ago

Closed 6 years ago

#203 closed defect (fixed)

Bug in optimization of range

Reported by: dagss Owned by: Prajwal Suhas P
Priority: critical Milestone: 0.11.1
Component: Python Semantics Keywords:
Cc:

Description

This:

   cdef int i, n
    n = 10
    for i in range(n):
        if i == 5: n *= 2
    print i

prints out 20.

Change History (11)

comment:1 Changed 6 years ago by dagss

It should print 9, I think

comment:2 Changed 6 years ago by dagss

Note: This must be changed in the for in range-optimization, because "for 0 <= i < n" should have this semantics!

comment:3 Changed 6 years ago by dagss

  • Milestone changed from 0.11 to 0.11.1

comment:4 Changed 6 years ago by dagss

  • Owner changed from somebody to Prajwal Suhas P

comment:5 Changed 6 years ago by robertwb

  • Resolution set to fixed
  • Status changed from new to closed

Works as expected. Committed http://hg.cython.org/cython-devel/rev/620326b14c3f

This is another reason to keep the for...from syntax though.

comment:6 Changed 6 years ago by robertwb

  • Component changed from Optimization to Python Semantics

comment:7 Changed 6 years ago by robertwb

This is a dupe of #196, the above commit was for #208.

comment:8 Changed 6 years ago by dagss

  • Resolution fixed deleted
  • Status changed from closed to reopened

No, this is a different bug. Reopening.

Also note that this ticket is assigned to Prajwal Suhas P; see http://thread.gmane.org/gmane.comp.python.cython.devel/4775

Magnus' patch was only for incrementing-one-to-far; this is about evaluating the endpoints to range once per loop.

Also consider this test case:

"""
 >>> f()
g called
0
1
2
2
"""

cdef int g():
     print "g called"
     return 3

def f():
     cdef int i
     for i in range(g()):
         print i
     print i

comment:9 Changed 6 years ago by dagss

That is; I get:

Failed example:
    f()
Expected:
    g called
    0
    1
    2
    2
Got:
    g called
    0
    g called
    1
    g called
    2
    g called
    2

comment:10 Changed 6 years ago by dagss

  • Priority changed from major to critical

comment:11 Changed 6 years ago by robertwb

  • Resolution set to fixed
  • Status changed from reopened to closed

Finally resolved by #243

Note: See TracTickets for help on using tickets.