Comments interleaved.
Magnus Lie Hetland wrote:
> Hi!
>
> I've put together a tiny patch for the for loop bug, and just thought
> I'd run it by you guys. As far as I understand, the "for ... from"
> node is also used for optimized range()-loops (i.e., over C integer
> indices) -- so that's the only place I fixed anything. (It seems to
> work for range too, now.)
>
This is correct. for in range(...) is transformed before the code
generation step into a forfrom.
> I've basically just mirrored the incop, creating an inverse named
> decop, to "undo" the last (superfluous) step, and then put a statement
> using this (such as "i--" if the incop is "i++", or "i-=2" if the
> incop is "i+=2") right after the closing parens of the for loop. As
> far as I can see, this should be safe, because if the code breaks out
> of the loop (using a goto in C), the last step will never occur, so
> the "undo" isn't needed.
>
> --- a/Cython/Compiler/Nodes.py Wed Dec 17 01:43:58 2008 -0800
> +++ b/Cython/Compiler/Nodes.py Fri Jan 30 12:02:15 2009 +0100
> @@ -3764,9 +3764,12 @@
> self.bound1.generate_evaluation_code(code)
> self.bound2.generate_evaluation_code(code)
> offset, incop = self.relation_table[self.relation1]
> + decop = "++" if incop == "--" else "--"
>
Cython supports Python 2.3, so you must change the syntax.
> if self.step is not None:
> + step = self.step.result()
> self.step.generate_evaluation_code(code)
> - incop = "%s=%s" % (incop[0], self.step.result())
> + incop = "%s=%s" % (incop[0], step)
> + decop = "%s=%s" % (decop[0], step)
> code.putln(
> "for (%s = %s%s; %s %s %s; %s%s) {" % (
> self.loopvar_name,
> @@ -3778,7 +3781,7 @@
>
> self.target.generate_assignment_code(self.py_loopvar_node, code)
> self.body.generate_execution_code(code)
> code.put_label(code.continue_label)
> - code.putln("}")
> + code.putln("} %s%s;" % (self.loopvar_name, decop))
> break_label = code.break_label
> code.set_loop_labels(old_loop_labels)
> if self.else_clause:
>
> I've basically just tested it with loops from 0 to 10 and from 10 to
> 0, with steps 1 and 2, with both loop syntaxes (for ... from and
> for ... in range(...)).
>
> Of course it's very possible that I've missed something obvious
> somewhere... :->
>
One minor issue: What if the iterator variable expression is a function?
I.e.:
cdef int getstep(): return 10
for 0 <= i < n by getstep():
...
Then self.step.result() will return a function call, and your decop will
invoke the function twice :-(
What you do is call "code.funcstate.allocate_temp(integer_type_of_step)"
to get a temporary variable which you can assign the step variable to,
and then you use this temporary instead. Finally, after the decrement
has happened, you call code.funcstate.release_temp(...) so that
code.funcstate can hand out the same temporary for the next for loop (or
whatever). See Code.py.
Dag Sverre
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev