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

Reply via email to