IronLanguages / ironpython3

Implementation of Python 3.x for .NET Framework that is built on top of the Dynamic Language Runtime.
Apache License 2.0
2.5k stars 287 forks source link

Dictionary lookup of class namespace should not apply to explicitly declared variables #1560

Closed BCSharp closed 2 years ago

BCSharp commented 2 years ago

A dictionary lookup behaviour is standard for classes. This means that any locally referenced name is first looked up in the class attribute dictionary (local namespace), before looked up in the enclosing lexical scopes. This is true even if the variable to be looked up is late-bound, e.g. introduced at runtime by modifying locals() or though metaclass __prepare__. This behaviour is already implemented in #1151 in general and in recent #1551 for metaclasses.

For instance:

prepared = "global"

class C:
    executed = prepared  # lexical name binding

assert C.executed == "global"

class Meta(type):
    @classmethod
    def __prepare__(metacls, name, bases):
        return {'prepared' : "meta"}

class C(metaclass=Meta):
    executed = prepared  # namespace dictionary lookup

assert C.executed == "meta"

However, when a variable is explicitly declared global, the default namebinding rules are overridden and the name has to resolve to a global variable:

prepared = "global"

class Meta(type):
    @classmethod
    def __prepare__(metacls, name, bases):
        return {'prepared' : "meta"}

class C(metaclass=Meta):
    global prepared
    executed = prepared  # explicit lexical name binding

assert C.executed == "global"

This is not how IronPython handles it currently.


An interesting story emerges when the variable is declared nonlocal rather than global. One (like me) would expect it too to override default namebinding rules and force lexical lookup. However, all CPython versions up to and including 3.10 still do local dictionary lookup:

class Meta(type):
    @classmethod
    def __prepare__(metacls, name, bases):
        return {'prepared' : "meta"}

def f():
    prepared = "f-local"
    class C(metaclass=Meta):
        nonlocal prepared
        executed = prepared  # which prepared?

    assert C.executed == "meta"  # ah, namespace dictionary lookup

f()

IronPython currently matches it, simply because none of this explicit declaration is taken into account yet. But as a programmer and IronPython developer I see it as a bug. Perhaps nobody reported it yet to CPython, or a report has been made, but got a very low priority. After all, this is a very unlikely piece of code that would reveal the bug. And arguably bad style.

Another demonstration that this is likely a bug rather a deliberate design is that references are resolved differently depending whether they are read (namespace lookup) or written (lexical lookup). To modify the example above a little:

class Meta(type):
    @classmethod
    def __prepare__(metacls, name, bases):
        return {'prepared' : "meta"}

def f():
    prepared = "f-local"
    class C(metaclass=Meta):
        nonlocal prepared
        prepared = "C-set"   # lexical lookup to nonlocal prepared
        executed = prepared  # namespace dictionary lookup, will not fetch "C-set"
    assert C.executed == "meta"
    assert prepared == "C-set"

f()

I'd prefer to implement the behaviour that seems correct, though feel uncomfortable deviating from CPython. But if I won't do it now, and then CPython one day changes its behaviour, who will remember the fix that now seems easy.

slozier commented 2 years ago

The nonlocal case is indeed odd, not what I would expect. Seems obscure enough that it's unlikely anyone would be relying on this behaviour. So, as long as it's not causing any CPython test failures, I'm fine with fixing it. We could have a test case pointing to the details (which could fail if CPython ever changes to match).