T490 - nonlocal support

[craigcitro, 2010-08-18 07:33:44]

So this changeset looks like two patches on top of one another -- a bunch of
refcounting-related stuff and support for nonlocal. I'd like to disentangle
those two, and make sure we've got testcases for whatever problems the
refcounting code fixes.

The nonlocal stuff looks good -- I had a few questions about how it interacts
with other stuff, and it wouldn't hurt to have a few more test cases, I think
... 

Nice work!

http://codereview.appspot.com/1291041/diff/2001/3001
File Cython/Compiler/ExprNodes.py (right):

http://codereview.appspot.com/1291041/diff/2001/3001#newcode1491
Cython/Compiler/ExprNodes.py:1491: code.put_gotref(self.result())
Just to check, are these two refcounting-related diffs anything to do with
nonlocal? Or fixes for a refcounting issue you ran into? If it's nothing to do
with nonlocal, we should probably pull these out into a separate commit and add
some test cases.

http://codereview.appspot.com/1291041/diff/2001/3002
File Cython/Compiler/Nodes.py (right):

http://codereview.appspot.com/1291041/diff/2001/3002#newcode1322
Cython/Compiler/Nodes.py:1322: code.put_var_giveref(entry)
Same comment from the refcounting changes in ExprNodes.py applies here. (In
particular, it'll be a lot easier to comment on them once I can stare at some
examples of bugs they fix.)

http://codereview.appspot.com/1291041/diff/2001/3002#newcode3221
Cython/Compiler/Nodes.py:3221: 
Just to check: does everything work fine with type inference? I suspect it does,
but it'd be nice to have a few test cases to double-check that. In particular,
if Cython decides to make some variable a C int, for example, and that gets used
by a nonlocal somewhere else, does that still work okay?

http://codereview.appspot.com/1291041/diff/2001/3003
File Cython/Compiler/Parsing.py (right):

http://codereview.appspot.com/1291041/diff/2001/3003#newcode981
Cython/Compiler/Parsing.py:981: return Nodes.NonlocalNode(pos, names = names)
According to PEP 3104, you can do "nonlocal x = 3" -- I don't think this code
will parse that, right? (Or is that only in the PEP and not implemented yet?)

http://codereview.appspot.com/1291041/diff/2001/3005
File Cython/Compiler/Symtab.py (right):

http://codereview.appspot.com/1291041/diff/2001/3005#newcode1221
Cython/Compiler/Symtab.py:1221: warning(pos, "'%s' redeclared  ", 0)
I think we might actually want to raise an error here -- according to this:

http://docs.python.org/dev/py3k/reference/simple_stmts.html#the-nonlocal-stat...

names used in a nonlocal statement can't collide with local names. However, I
must be confused, because this definition:


def f():
  x = 1
  def g():
    x = 3
    nonlocal x
    x = 5
    return x
  print(g())
  print(x)

only gives a warning, not an error. However, including x as a parameter for g
does give an error, as expected ...

http://codereview.appspot.com/1291041/diff/2001/3007
File tests/run/nonlocal_T490.pyx (right):

http://codereview.appspot.com/1291041/diff/2001/3007#newcode66
tests/run/nonlocal_T490.pyx:66: 
These tests look good, but I think we also definitely need some *illegal* uses
of nonlocal. (There are probably other legal variations on nonlocal we should
try out, but those are less pressing.)