Skip to content

Range optimization has semantical issues #565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
pfalcon opened this issue May 4, 2014 · 3 comments
Closed

Range optimization has semantical issues #565

pfalcon opened this issue May 4, 2014 · 3 comments
Labels
bug

Comments

@pfalcon
Copy link
Contributor

pfalcon commented May 4, 2014

Here's example:

n = 20

for i in range(n):
    if i == 3:
        n = 1
    print(i)

On uPy, this ends loop prematurely, while expected semantics of range is that it captures value it was initialized with, not reference to variable.

But what really made me look into this is the gross performance difference of using range with constant expression vs one with var as arg. My guess proper semantics would be to load the end value on stack at the beginning of iteration and use that afterwards...
@lurch
Copy link
Contributor

lurch commented May 4, 2014

A temporary workaround would be to change it to:

n = 20

for i in iter(range(n)):
    if i == 3:
        n = 1
    print(i)

;-)

@dpgeorge
Copy link
Member

For reference, the following also has issues (when the iterated variable is written to):

for i in range(10):
    print(i)
    i = 10

dpgeorge added a commit that referenced this issue Dec 11, 2014
@dpgeorge
py: Fix a semantic issue with range optimisation.
c33ce60 
Now you can assign to the range variable within the for loop and it will
still work.

Partially addresses issue #565.
dpgeorge added a commit that referenced this issue Dec 12, 2014
@dpgeorge
py: Fix optimised for-loop compiler so it follows proper semantics.
e181c0d 
You can now assign to the range end variable and the for-loop still
works correctly.  This fully addresses issue #565.

Also fixed a bug with the stack not being fully popped when breaking out
of an optimised for-loop (and it's actually impossible to write a test
for this case!).
@dpgeorge
Copy link
Member

Optimised for-loop now has correct semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@lurch @pfalcon @dpgeorge