On Sep 25, 2008, at 12:54 PM, Dag Sverre Seljebotn wrote: > Robert wrote: >> On Sep 24, 2008, at 3:09 PM, Dag Sverre Seljebotn wrote: >>> See: >>> >>> http://hg.cython.org/cython-dagss/rev/28b0311d6bff >> >> I think this is a nice approach that could clean up a lot of stuff. >> (In particular, I think we could get rid of almost every CloneNode >> and the (admittedly somewhat hackish) PersistentNode.) The one thing >> I would change is having to refer to these handles by index into a >> list passed at creation time. I would rather have an interface >> something like >> >> tempblock = TempsBlockNode(pos, body=None) >> foo = tempblock.new_temps(PyrexTypes.c_int_type) >> a, b = tempblock.new_temps(PyrexTypes.py_object_type, >> PyrexTypes.py_object_type) >> tempblock.body = SingleAssignmentNode(pos, lhs=foo.ref(), >> rhs=node) >> >> Actually, it does feel a bit weird setting the body after it is >> created. What about >> >> foo = TempRefNode(PyrexTypes.c_int_type) >> body = SingleAssignmentNode(pos, lhs=foo.ref(), rhs=node) >> return TempsBlockNode(pos, body=body, temps=[foo]) > > I didn't like it either, but it was the best I came up with there > and then. > > The problem I have with your suggestions is this: I took care to > make a) > the temporary and b) a node in the tree referring to a temporary, > seperate > things.
Sorry, I had a really bad typo in my example. What I meant was "foo = TempHandle(PyrexTypes.c_int_type)" > Even if node.pos is made optional/ignored (which could vary among > TempRefNodes referring to the same temp), I think it makes sense to > keep > this distinction -- for instance, control flow analysis might want > to tag > possible variable values to TempRefNode instances which would only > hold > about the temp at that location. It just doesn't feel right to have > a node > sit in more than one place in the tree... Yes. > So, what about just being explicit and do: > > foo = TempHandle(type) > a = TempRefNode(pos, foo) > b = TempRefNode(pos, foo) > ... TempsBlockNode(pos, temps=[foo]) > > ? Yes. That's what I meant to say :). I was thinking foo.ref() as shorthand for TempRefNode(pos, foo). I guess we should pas pos in as well. >> P.S. When I mentioned incremental temp migration, >> FunctionState.allocate_temp is exactly what I had in mind. Nice. > > Yes, that was don in in summer. Once everything is moved over in a > year or > so we can go back to only one temp namespace. I'd forgotten about that (or didn't realize it at the time--I read a lot of code but didn't go through it all with a fine-toothed comb :). - Robert _______________________________________________ Cython-dev mailing list [email protected] http://codespeak.net/mailman/listinfo/cython-dev
