Stefan Behnel wrote: > Hi Dag, > > Dag Sverre Seljebotn wrote: > >> Be aware of the TempsBlockNode I wrote if you look at this, for any >> temporary variables needed. (It is currently unused so may have bugs >> too...ask if you have problems with it) >> > > Yep, I found it already. :) > > The current implementation works for me in all cases I found relevant. It > supports all three .iter*() methods and .iteritems() works with both a > tuple and two separate variables as targets. > > However, the code (in Optimize.py) looks huge and feels a bit clumsy. Could > you review it and try to fix it up? > I think it looks OK to me, mostly, but see below. These kind of long code strings is what you get when you manually construct nodes -- one could work at making node construction more readable, or you could try to use a TreeFragment (with three different cases depending on the unpacking). It would be ok to simply have a FOO call-node and then replace the SimpleCallNode's entry after the TreeFragment construction. But within the current framework seems to be the kind of code length you get.
There's one thing that worries me: Calling TupleNode's allocate_temps. The reason is that "old" temp allocation is tightly connected to the order things happen in (a temp is "locked" for the duration of the recursive call), and must NOT be interfered with from any "external" code path (which I why I want to get rid of it and move to the new one). TupleNode doesn't allocate temps directly, but one also needs to be sure that the children does not do any temp allocation. (Passing in "None" as the env parameter certainly helps enforce this. I.e. if allocate_temps does something that accesses the "env" parameter, then you probably cannot do it at this stage without problems.) These things lead to extremely subtle bugs (i.e. reuse of temporary variables in wrong places), so care is needed. Ideally, one should find a way of not having to call analyse_types and allocate_temps on TupleNode, instead one should pass the different variables that is set during analysis directly to the TupleNode constructor (i.e. the TupleNode must be constructed in an "analysed state"). I don't have time for fixing this up right now though. Dag Sverre _______________________________________________ Cython-dev mailing list [email protected] http://codespeak.net/mailman/listinfo/cython-dev
