Skip to content

Commit 3ff2d03

Browse files
dpgeorgedpgeorge
committed
py: Fix bug in optimised for .. range.
Don't store final, failing value to the loop variable. This fix also makes for .. range a bit more efficient, as it uses less store/load pairs for the loop variable.
1 parent 8cd72bd commit 3ff2d03

3 files changed

Lines changed: 183 additions & 7 deletions

File tree

‎py/compile.c‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,35 +1534,41 @@ void compile_while_stmt(compiler_t *comp, mp_parse_node_struct_t *pns) {
15341534
}
15351535

15361536
// TODO preload end and step onto stack if they are not constants
1537-
// TODO check if step is negative and do opposite test
1537+
// Note that, as per semantics of for .. range, the final failing value should not be stored in the loop variable
1538+
// And, if the loop never runs, the loop variable should never be assigned
15381539
void compile_for_stmt_optimised_range(compiler_t *comp, mp_parse_node_t pn_var, mp_parse_node_t pn_start, mp_parse_node_t pn_end, mp_parse_node_t pn_step, mp_parse_node_t pn_body, mp_parse_node_t pn_else) {
15391540
START_BREAK_CONTINUE_BLOCK
15401541

15411542
int top_label = comp_next_label(comp);
15421543
int entry_label = comp_next_label(comp);
15431544

1544-
// compile: var = start
1545+
// compile: start, duplicated on stack
15451546
compile_node(comp, pn_start);
1546-
c_assign(comp, pn_var, ASSIGN_STORE);
1547+
EMIT(dup_top);
15471548

15481549
EMIT_ARG(jump, entry_label);
15491550
EMIT_ARG(label_assign, top_label);
15501551

1552+
// at this point we actually have 1 less element on the stack
1553+
EMIT_ARG(set_stack_size, EMIT(get_stack_size) - 1);
1554+
1555+
// store next value to var
1556+
c_assign(comp, pn_var, ASSIGN_STORE);
1557+
15511558
// compile body
15521559
compile_node(comp, pn_body);
15531560

15541561
EMIT_ARG(label_assign, continue_label);
15551562

1556-
// compile: var += step
1557-
c_assign(comp, pn_var, ASSIGN_AUG_LOAD);
1563+
// compile: var + step, duplicated on stack
1564+
compile_node(comp, pn_var);
15581565
compile_node(comp, pn_step);
15591566
EMIT_ARG(binary_op, MP_BINARY_OP_INPLACE_ADD);
1560-
c_assign(comp, pn_var, ASSIGN_AUG_STORE);
1567+
EMIT(dup_top);
15611568

15621569
EMIT_ARG(label_assign, entry_label);
15631570

15641571
// compile: if var <cond> end: goto top
1565-
compile_node(comp, pn_var);
15661572
compile_node(comp, pn_end);
15671573
assert(MP_PARSE_NODE_IS_SMALL_INT(pn_step));
15681574
if (MP_PARSE_NODE_LEAF_SMALL_INT(pn_step) >= 0) {
@@ -1572,6 +1578,9 @@ void compile_for_stmt_optimised_range(compiler_t *comp, mp_parse_node_t pn_var,
15721578
}
15731579
EMIT_ARG(pop_jump_if_true, top_label);
15741580

1581+
// discard final value of var that failed the loop condition
1582+
EMIT(pop_top);
1583+
15751584
// break/continue apply to outer loop (if any) in the else block
15761585
END_BREAK_CONTINUE_BLOCK
15771586

‎tests/basics/for2.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
i = 'init'
2+
for i in range(0):
3+
pass
4+
print(i) # should not have been modified
5+
6+
for i in range(10):
7+
pass
8+
print(i) # should be last successful value of loop

‎tests/misc/features.py‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
# mad.py
2+
# Alf Clement 27-Mar-2014
3+
#
4+
zero=0
5+
three=3
6+
print("1")
7+
print("2")
8+
print(three)
9+
print("{}".format(4))
10+
five=25/5
11+
print(int(five))
12+
j=0
13+
for i in range(4):
14+
j += i
15+
print(j)
16+
print(3+4)
17+
try:
18+
a=4/zero
19+
except:
20+
print(8)
21+
print("xxxxxxxxx".count("x"))
22+
def ten():
23+
return 10
24+
print(ten())
25+
a=[]
26+
for i in range(13):
27+
a.append(i)
28+
print(a[11])
29+
print(a[-1])
30+
str="0123456789"
31+
print(str[1]+str[3])
32+
def p(s):
33+
print(s)
34+
p("14")
35+
p(15)
36+
class A:
37+
def __init__(self):
38+
self.a=16
39+
def print(self):
40+
print(self.a)
41+
def set(self,b):
42+
self.a=b
43+
a=A()
44+
a.print()
45+
a.set(17)
46+
a.print()
47+
b=A()
48+
b.set(a.a + 1)
49+
b.print()
50+
for i in range(20):
51+
pass
52+
print(i)
53+
if 20 > 30:
54+
a="1"
55+
else:
56+
a="2"
57+
if 0 < 4:
58+
print(a+"0")
59+
else:
60+
print(a+"1")
61+
a=[20,21,22,23,24]
62+
for i in a:
63+
if i < 21:
64+
continue
65+
if i > 21:
66+
break
67+
print(i)
68+
b=[a,a,a]
69+
print(b[1][2])
70+
print(161//7)
71+
a=24
72+
while True:
73+
try:
74+
def gcheck():
75+
global a
76+
print(a)
77+
gcheck()
78+
class c25():
79+
x=25
80+
x=c25()
81+
print(x.x)
82+
raise
83+
except:
84+
print(26)
85+
print(27+zero)
86+
break
87+
print(28)
88+
k=29
89+
def f():
90+
global k
91+
k = yield k
92+
print(next(f()))
93+
while True:
94+
k+= 1
95+
if k < 30:
96+
continue
97+
break
98+
print(k)
99+
for i in [1,2,3]:
100+
class A():
101+
def __init__(self, c):
102+
self.a = i+10*c
103+
b = A(3)
104+
print(b.a)
105+
print(34)
106+
p=0
107+
for i in range(35, -1, -1):
108+
print(i)
109+
p = p + 1
110+
if p > 0:
111+
break
112+
p=36
113+
while p == 36:
114+
print(p)
115+
p=37
116+
print(p)
117+
for i in [38]:
118+
print(i)
119+
print(int(exec("def foo(): return 38") == None)+foo())
120+
d = {}
121+
exec("def bar(): return 40", d)
122+
print(d["bar"]())
123+
def fib2(n):
124+
result = []
125+
a, b = 0, 1
126+
while a < n:
127+
result.append(a)
128+
a, b = b, a+b
129+
return result
130+
print(fib2(100)[-2]-14)
131+
Answer={}
132+
Answer["ForAll"]=42
133+
print(Answer["ForAll"])
134+
i = 43
135+
def f(i=i):
136+
print(i)
137+
i = 44
138+
f()
139+
print(i)
140+
while True:
141+
try:
142+
if None != True:
143+
print(45)
144+
break
145+
else:
146+
print(0)
147+
except:
148+
print(0)
149+
print(46)
150+
print(46+1)
151+
def u(p):
152+
if p > 3:
153+
return 3*p
154+
else:
155+
return u(2*p)-3*u(p)
156+
print(u(16))
157+
def u49():
158+
return 49
159+
print(u49())

0 commit comments

Comments
 (0)