Bug when shadowing a global variable with a local one

[vnmabus]

First, I have to say that I found this bug when using Chameleon 3.3, and I was able to reproduce it also in 3.10. I did not try more modern versions because I triggered this in code that still uses Python 2.7, which is not supported in Chameleon 4.x. However, looking at the code in the master branch I think this bug has not been fixed yet.

Description of the problem

Suppose that a variable foo is defined globally. Then, in some code block, we define the variable foo again as a local variable. What should happen then?

This case is not 100% clear from the docs. However, what one would expect from other programming languages, and from the original implementation of ZPT is that the local variable definition "shadows" or hides the global one, inside the scope where the local variable "lives".

That is what happens at a first glance in Chameleon too. However, if a macro is invoked inside the scope of the local variable, when the macro returns, the local variable is updated with the value of its global version (even if the value is not changed inside the macro), which is unexpected:

chameleon/src/chameleon/compiler.py

Line 1685 in 90b9002

template("econtext.update(rcontext)")

What one would expect instead is that the values are only updated for variables modified inside the macro (thus, only restoring the global value when one leaves the scope of the local redefinition).

[malthe]

You mean global as in tal:define="global <name> <expression>" – ?

It might be a bug, but I would say generally speaking, don't redefine globals.

[vnmabus]

Yes. As mentioned above, in the original ZPT implementation, and in most programming languages, one can create a local variable with the same name as a global one, which shadows the global one in its scope. The alternative would be to require that the developer remembers all the global variables he has ever defined, even when they are not relevant to the current code, in order to not reuse these names.

In Chameleon it "almost" works (it works if you do not call a macro inside the local variable scope, as returning from the macro sets the value of the variable to the global one). Thus, yes, I would consider that a bug.

If defining a local variable with the same name as a global one is supposed to not be supported, then I would detect that and raise an exception. Otherwise the code will seem to work until you refactor some part into a macro and you hit difficult to debug errors. My personal preference, of course, would be to fix the current behavior so that shadowing globals works.

[malthe]

Oh I don't disagree it's a bug, sorry if that was not clear, just that it might not be trivial to fix and so in the short-term, a workaround is "don't do that".