Repository navigation
erroneous behavior when creating classes inside a closure #53472
Description
Activity
monsanto commented
on Jul 11, 2010 monsantomannequinMannequinAuthorI have a function whose closure contains a local variable that shadows a global variable (lets call it x). If I create a class as follows:
class Test(object): x = x
Test.x will contain the value of the global x, not the local x. This ONLY happens when the names are the same, and it only happens in the class body; i.e., "class Test(object): y = x" and class "Test(object): pass; Test.x = x" work fine.
However, if there is an assignment x = x AND you make other assignments, such as y = x, in the body, the other variables will have the wrong value too.
Test case attached. Problem noticed on Python 2.6.2 on Windows and 2.6.5 on Linux.
monsanto commented
on Jul 11, 2010 monsantomannequinMannequinAuthorA friend confirmed that this was the case on 3.1.2 as well.
I'm not sure what I correct behavior is in this case. Consider the function equivalent:
x = 3 def f(x): def m(): x = x print x m() f(4)
which gives:
Traceback (most recent call last): File "x.py", line 7, in <module> f(4) File "x.py", line 6, in f m() File "x.py", line 4, in m x = x UnboundLocalError: local variable 'x' referenced before assignment
The class example works because name namespaces are unoptimized, so failing to find a binding in the local (class) namepsace, Python looks at the globals and finds the global definition.
I don't see anything in
http://docs.python.org/reference/executionmodel.html#naming-and-binding
to suggest that the class should behave differently from a nested function here; that is, I'd expect UnboundLocalError.
Jython 2.5.1 gives the same results as Python:
newton:~ dickinsm$ cat test.py
x = "error"def test(x): class Test(object): x = x print("x: ", x) print("Test.x: ", Test.x) test("success") newton:~ dickinsm$ jython2.5.1/jython test.py ('x: ', 'success') ('Test.x: ', 'error')
I agree with Mark, I'd expect an UnboundLocalError. I remembered this thread on Python-dev that may or may not be relevant, but is certainly analogous:
http://www.mail-archive.com/python-dev@python.org/msg37576.html
Here's a patch. It raises a NameError in that case.
I think it would be worth bringing this up on python-dev, especially since it affects alternative Python implementations.
It would also be good to have a documentation fix to the reference manual that clearly explains whatever behaviour is decided on; it's not at all clear (to me, anyway), how to extract this information from the docs.
Chris, when posting something like this, *please* include the output. I had to insert ()s to run this with 3.1. I will upload the py3 version as test3.py. Is your output the same as mine?
x: success
Test.x: error
Test2.y: success
Test3.x: error
Test3.y: error
Test4.x: successThere is an obvious inconsistency between Test2 and Test/Test3. This shows up also in the dis.dis(test) output. So there is definitely a bug.
To me, the Test2 result is the error. I base this on 7.7 Class Definitions: "The class’s suite is then executed in a new execution frame (see section Naming and binding), using a newly created local namespace and the original global namespace." I interpret this to mean that intermediate namespaces are not used (as was the case before 2.2). Indeed, this sentence is unchanged from the 2.1 doc (and before).
http://docs.python.org/release/2.1/ref/class.htmlOf course, the intent could have changed without changing the wording, by reference to the Naming and Binding section, but then this sentence really should be changed too.
The current Naming and Binding section includes:
"A scope defines the visibility of a name within a block. If a local variable is defined in a block, its scope includes that block. If the definition occurs in a function block, the scope extends to any blocks contained within the defining one, unless a contained block introduces a different binding for the name. The scope of names defined in a class block is limited to the class block; it does not extend to the code blocks of methods."
So class blocks are an exception in propagating down, and I thought they were also an exception for propagating into, for the reason stated above.
It is possible that this is an undefined corner of the language. Certainly, the compiler is confused as it treats one nested class (Test2) as a closure and the other three nested classes as not.
Since the name and binding design is Guido's and central to Python's operation, I personally would not touch it without his input. Hence I have added him as nosy and second the idea of pydev discussion.
Hm. This seems an old bug, probably introduced when closures where first introduced (2.1 ISTR, by Jeremy Hylton).
Class scopes *do* behave differently from function scopes; outside a nested function, this should work:
x = 1 class C(object): x = x assert X.x == 1
And I think it should work that way inside a function too.
So IMO the bug is that in classes Test and Test3, the x defined in the function scope is not used. Test2 shows that normally, the x defined in the inner scope is accessed.
So, while for *function scopes* the rules are "if it is assigned anywhere in the function, every reference to it references the local version", for *class scopes* (outsided methods) the lookup rules are meant to be dynamic, meaning "if it isn't defined locally yet at the point of reference, use the next outer definition".
I haven't reviewed the patches.
I meant, of course,
assert C.x == 1
Guido clarified:
Class scopes *do* behave differently from function scopes;
outside a nested function, this should work:x = 1 class C(object): x = x assert C.x == 1And I think it should work that way inside a function too.
I take that to mean that
x = 0 def f() x = 1 class C(object): x = x assert C.x == 1 f()
should work, meaning that C.x==0 and UnboundLocalError are both wrong.
That would mean to me that in "The class’s suite is then executed in a new execution frame (see section Naming and binding), using a newly created local namespace and the original global namespace." the phrase "the original global namespace" should be changed to "the surrounding namespaces".
I also think this from Guido
"So, while for *function scopes* the rules are "if it is assigned anywhere in the function, every reference to it references the local version", for *class scopes* (outsided methods) the lookup rules are meant to be dynamic, meaning "if it isn't defined locally yet at the point of reference, use the next outer definition"."
should somehow also be clearer, probably also in the class page, so that people will neither expect an UnboundLocalError.
On Sat, Jul 24, 2010 at 3:21 PM, Terry J. Reedy <report@bugs.python.org> wrote:
Terry J. Reedy <tjreedy@udel.edu> added the comment:
Guido clarified:
> Class scopes *do* behave differently from function scopes;
> outside a nested function, this should work:
> x = 1
> class C(object):
> x = x
> assert C.x == 1
> And I think it should work that way inside a function too.I take that to mean that
x = 0
def f()
x = 1
class C(object):
x = x
assert C.x == 1
f()should work, meaning that C.x==0 and UnboundLocalError are both wrong.
Indeed.
That would mean to me that in "The class’s suite is then executed in a new execution frame (see section Naming and binding), using a newly created local namespace and the original global namespace." the phrase "the original global namespace" should be changed to "the surrounding namespaces".
Those words sound like they were never revised since I wrote them for
Python 0.9.8 or so...I also think this from Guido
"So, while for *function scopes* the rules are "if it is assigned anywhere in the function, every reference to it references the local version", for *class scopes* (outsided methods) the lookup rules are meant to be dynamic, meaning "if it isn't defined locally yet at the point of reference, use the next outer definition"."
should somehow also be clearer, probably also in the class page, so that people will neither expect an UnboundLocalError.
FWIW, unless something drastically changed recently, the language
reference is likely out of date in many areas. I would love it if a
team of anal retentive freaks started going through it with a fine
comb so as to make it describe the state of the implementation(s) more
completely.11 remaining items
Reproduced on main (3.15).
I want to try taking this.
Sergey, I don't know if you got a chance to look at this issue. I think we need two changes, one more complicated than the other. The symbol table needs to track references to variables in the class scope and potentially identify bindings to non-globals. This change shouldn't be too significant, because that context already flows around class namespaces, because methods can have free variables bound to enclosing scopes. The more significant change is that LOAD_NAME only knows how to read locals from the dict. It doesn't know how to reference cells. Worse it gets passed a string, and it won't know how to interpret that string at runtime.
The class namespace is complicated. The current implementation allows a variable to switch between local and global dynamically.
a = 1 class C: x = a # a refers to the global a = a + 1 # now a is a local y = a del a # the local is gone, so a is a global again z = a assert C.x == 1 assert C.y == 2 assert C.z == 1If we want this switching semnatics to be preserved for a class defined inside a function, then any variable referenced in the class and also defined in an enclosing scope must be a cell variable at its definition site and pass through to the class. We can't guarantee that the variable is current live as a local at the time it is used.
Is that the desired intent?
Hm. So if you move things into a function you get something like this:
def outer(a): class C: x = a # outer a a = 42 # now a is local in class namespace y = a del a z = a # outer a again return C
That should work, whereas currently it fails with a
NameErroronx = a. I guess the implementation should use a special read operation that tries the class-local first and falls back to the cell (and no further). Write operations always set the class-local.We can use static analysis to find an outer
a, but not to find the class-localaor a globala-- those are always handled completely dynamically. (I'm not sure if there's a way to set a class-local using a dynamic API that static analysis would miss, but I suspect there might be a way. Also, this is how things work currently.)Static analysis should also find an
aif it's several function scopes away. However if there's another class scope surrounding all this (e.g.outeris itself a method of some class) that class namespace is skipped, as usual.PS. I thought there was a special opcode that did something related (something with a cell and a dict entry) but I can't find it.
PPS. What should happen if you were to put
nonlocal ain the class body? The logical answer seems to be that it skips looking in the class-local namespace and goes straight to the cell (and there must be a cell -- no fallback to globals).PPPS. I think my opinion hasn't changed since 2010. :-)
A version of this change was implemented by #103764 to support PEP 695.
https://jellezijlstra.github.io/pep695#class-scopes describes the implementation in more detail.It looks like one thing that didn't get changed here is the language spec, which still says that unbound variables in the class scope are looked up in the global namespace: "These references follow the normal rules for name resolution with an exception that unbound local variables are looked up in the global namespace."
I'll slow down a little after clarifying my previous comment. The recent change affects free variables in a class body, but not "local" variables. If you assigned to a variable in the class body, the classic behavior of LOCALS & GLOBALS is maintained. If you refer to a variable that is not assigned to, then it changes to refer to name bindings in an enclosing scope. If I'm following that correctly, then this is a bit inconsistent. We should have local variables and free variables resolved using the same semantics.
Oh, so this uses dynamic lookup in class + globals:
def outer(a): class C: a = 42 x = a # 42; uses LOAD_NAME del a y = a # raises NameError
but this example references the outer a:
def outer(a): class C: x = a # outer a; uses LOAD_FROM_DICT_OR_DEREF
and this fails on the
print(a):def outer(a): class C: print(a) # raises NameError; uses LOAD_NAME a = 42
and this doesn't work:
def outer(a): class C: a = a # raises NameError; uses LOAD_NAME
(All the assignments use STORE_NAME.)
The difference between LOAD_NAME and LOAD_FROM_DICT_OR_DEREF is as follows:
- LOAD_NAME looks in the class scope and falls back on the global scope.
- LOAD_FROM_DICT_OR_DEREF looks in the class scope and in a cell (for a variable defined in a containing function scope).
The bytecode compiler generates LOAD_FROM_DICT_OR_DEREF iff:
- there is a containing function scope defining the name, and
- there is no assignment anywhere in the class scope to that name.
@JelleZijlstra OOC did you intend to change this behavior as part of PEP 695? We should update the language reference to reflect the behavior that is currently implemented. We may want to write a PEP that considers explicitly whether the current behavior should be preserved, or something closer to the semantics proposed here should be implemented.
I didn't intend to change any behavior for existing code in the PEP 695 implementation, and I don't think I did. This issue's original example works the same in all recent versions:
$ for python in 3.9 3.10 3.11 3.12 3.13 3.14; do uv run --python=$python python -c '''x = 1 def f(x): class Test: x = x return Test.x print(f(2))'''; done 1 1 1 1 1 1I haven't fully read the issue, but my view is that class namespaces have some surprising behaviors, but they do follow a set of rules that makes sense if you follow the logic closely enough, and we can't realistically afford to change any such behaviors.
Thanks for clarifying. It does look like this fell out of sync with the language spec a long time ago.
Apparently we discussed this a long time ago
The implementation changed in Python 3.4:
3b0431d
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: