This is a summary post, with a couple of answers to Greg and Robert.

You'll notice I've taken your side now, so this hopefully rounds off the 
discussion, this is just a last explanation round. Especially since this 
thread will be the canonical thread to look up for such matters in the 
future.

Robert Bradshaw wrote:
> 
> I think simply moving everything over to the code generation phase  
> will be the most beneficial. This will make things much more natural-- 
> when one needs a temp one can request it right there, use it, and  
> dispose of it when done. The "generate_disposal_code" paradigm makes  

I think the problem with generate_disposal_code in pure principle is 
that the things it does can't in reality be bundled into a nice disposal 
package:

- There's actually two different disposal_code methods depending on the 
situation of the parent
- Parents of an ExprNode now and then are seen to manually meddle; 
perhaps check the is_temp status and take action accordingly and so on.
- When DECREF is inserted in addition by return stats and except 
clauses, one could look at it as disposal_code being reimplemented for 
that particular situation.

So, I'd prefer a *declarative* scheme (I'm counting Stefan's Target 
class as such one), where the node exports both its result, and what 
needs to be done with it afterwards (refcount status/temp 
status/anything else). This makes the boundaries somewhat harder and the 
flow a little bit simpler -- leading to easier unit testing, it being 
more natural to handle things in return stats and except stats., etc.

Note that this is philosophical objections to design, it works this way, 
and I'm not advocating a change now since I got more working. As in, I 
think it is likely there will never be a change, as long as the rest of 
the nodes can be gracefully converted. (Keep in mind that there's 
certain things the allocate_temps phase did, like the "result" parameter 
and the possibility for the parent to direct the flow of temps in 
creative ways, for which alternative/similar solutions must be found.)

Greg Ewing wrote:
 > Dag Sverre Seljebotn wrote:
 >
 >> def generate_assignment_code(self, rhs, code):
 >>      if not rhs.can_raise_exception:
 >>          # No temp needed
 >>          if self.type.is_pyobject:
 >>              code.put_decref(self.cname, self.type)
 >>          rhs.preparation_code(code)
 >>          rhs.store_result_code(self.cname, code)
 >
 > This isn't safe. A new reference to the rhs must be obtained
 > first in case it's the same as the old value of the lhs,
 > otherwise you risk losing the object.
 >

Ahh.

I'm horrible with refcounting. Certainly makes me humble about 
rebuilding anything; your experience in such things which is embedded 
into the current system is a big reason to keep it.

Greg Ewing wrote:
 > Dag Sverre Seljebotn wrote:
 >
 >> With e.g. x = ((((a + b) + c) + d) + e), and also the reverse nesting,
 >> the theoretical optimum is 1 temporary.
 >
 > What code do you have in mind to evaluate this using
 > only one temp? Seems to me you need two, used alternately.
 >

Yes. I definitely thought I'd found a solution with 1, but now I think 
there must be 2.

Greg Ewing wrote:
 > Dag Sverre Seljebotn wrote:
 >
 >> Without doing this, and *only* move the temp allocation phase to code
 >> generation time, the former code (the seperate allocate_temp phase) is
 >> still needed
 >
 > I don't follow that. The only reason I added a third phase
 > for temp allocation was because it had to be done after
 > inserting coercion nodes. I can't think of any reason it
 > couldn't be fully merged into the code generation pass.
 > Temps can be allocated by generate_evaluation_code and
 > freed by generate_disposal_code.
 >

Mainly I misunderstood what you said, and so my answer didn't make sense 
to you.

You'll find that this is more or less exactly what I've done with 
NewTempExprNode now, and it is what I go for now.

-- 
Dag Sverre
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev

Reply via email to