Generated Python code scopes variables incorrectly

[GoogleCodeExporter]

Since (if I'm not mistaken) all Blockly variables are global, the generated 
Python code incorrectly scopes variables locally if they are assigned.

What steps will reproduce the problem?
Build the following Blockly program:

  procedure setx
    set x "Hello"

  procedure printx
    print (get x)

  call setx
  call getx

Running this program displays "Hello".

What is the expected output? What do you see instead?
This generates the following Python program:

  x = None

  def setx():
    x = 'Hello'

  def printx():
    print x

  setx()
  printx()

Running this code prints "None". That is because in setx, x is a local variable 
because it is assigned. (Note that in printx, x is correctly treated as a 
global variable because it is not assigned.)

The Python code generator needs to add the 'global' statement for any variable 
assigned in a procedure. For example:

  def setx():
    global x
    x = 'hello'

What version of the product are you using? On what operating system?
SVN r228. Mozilla Firefox 13.

Original issue reported on code.google.com by matt.gi...@gmail.com on 10 Jun 2012 at 4:03

[GoogleCodeExporter]

Attached an XML file of the example program mentioned above.

Original comment by matt.gi...@gmail.com on 10 Jun 2012 at 4:06

Attachments:

• scoping.xml

[GoogleCodeExporter]

Added patch to fix issue.

Note that this creates a global statement for any variable that is a) assigned, 
b) used in a 'change _ by _' statement, or c) used as the control variable of a 
for or for-each loop.

Original comment by matt.gi...@gmail.com on 10 Jun 2012 at 5:49

Attachments:

• python-globals.patch

[GoogleCodeExporter]

Great bug, greater patch! Modified it slightly so potential new blocks will be 
covered. Your patch is superior in that it only makes global any assigned 
variables (in setters), whereas ours catches all variables, even just getters. 
For the educational environment, we feel that consistency is better.

Original comment by Q.Neut...@gmail.com on 10 Jun 2012 at 7:23

• Changed state: Fixed

[GoogleCodeExporter]

Cool, thanks for accepting. One thing is I'm not sure what you mean by 
"consistency is better."

Are you talking about consistency in the behaviour of the Python program when 
it runs? Or that it's simply better to list all variables in 'globals' for 
clarity (regardless of whether it is redundant)?

Because if you mean the latter, I take your point. But if you mean the former, 
then please note that my patch is equivalent in terms of runtime behaviour: if 
a variable is not assigned in a function, then Python will implicitly treat it 
as global, so there is no need to explicitly label it as 'global'. So using my 
example above, your patch will mean that 'global x' appears in the printx 
function, even though it is unnecessary to do so.

But I do take your point: it is much less likely to break in the future if you 
simply declare all variables as global instead of trying to be clever as I was.

Original comment by matt.gi...@gmail.com on 10 Jun 2012 at 7:40

[GoogleCodeExporter]

We were thinking of consistency for users.  Imagine a user sees this code:

def foo():
  global x
  x += 1
  print y

Then they add "y += 1" to the generated Python function and wonder why it just 
blew up.  That's weird even for a seasoned programmer who's unfamiliar with 
Python's whacky/awesome/fearsome scope system.  Just pretending that all 
variables need to be listed is simpler to understand.

Original comment by neil.fra...@gmail.com on 10 Jun 2012 at 7:47

[GoogleCodeExporter]

Right, got you. That makes sense.

Original comment by matt.gi...@gmail.com on 10 Jun 2012 at 8:55