classification
Title: Use of super overwrites use of __class__ in class namespace
Type: behavior Stage: needs patch
Components: Interpreter Core Versions: Python 3.4, Python 3.3, Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, Mark.Shannon, alex, barry, benjamin.peterson, carsten.klein@axn-software.de, cvrebert, daniel.urban, eric.snow, meador.inge, michael.foord, ncoghlan, python-dev
Priority: normal Keywords: needs review, patch

Created on 2011-06-19 22:28 by michael.foord, last changed 2013-05-15 20:28 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
class_super_1.patch daniel.urban, 2012-11-26 17:20 class stmt creates two scopes, the outer contains __class__ review
Messages (23)
msg138670 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-06-19 22:28
In Python 3 the following code prints "False" because the use of super() has caused the __class__ descriptor to be omitted from the class namespace. Remove the use of super and it prints "True".


class X(object):
    
    def __init__(self):
        super().__init__()
    
    @property
    def __class__(self):
        return int
        
print (isinstance(X(), int))
msg138677 - (view) Author: Roundup Robot (python-dev) Date: 2011-06-20 00:45
New changeset 2d62ee4e7d98 by Benjamin Peterson in branch 'default':
use a invalid name for the __class__ closure for super() (closes #12370)
http://hg.python.org/cpython/rev/2d62ee4e7d98
msg138712 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-06-20 13:49
And to record the workaround for 3.1 and 3.2 (courtesy of Michael):

Adding a "_super = super" alias at the module level and using the Python 2.x style long form invocation on _super() in affected methods will avoid the compiler games played when using super() directly. That is::

_super = super
class X(object):
    
    def __init__(self):
        _super(self, X).__init__()
    
    @property
    def __class__(self):
        return int
        
print (isinstance(X(), int))
msg138713 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2011-06-20 13:50
That work around seems ugly.  Why not back port the fix?  It doesn't seem like it could break anything and it's not even arguably a new feature, right?
msg138714 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2011-06-20 13:52
One reason is that it bumps the pyc magic number.
msg138720 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2011-06-20 14:15
Ah okay, I didn't see that in the changeset.
msg161703 - (view) Author: Roundup Robot (python-dev) Date: 2012-05-27 08:17
New changeset 96ab78ef82a7 by Nick Coghlan in branch 'default':
Close #14857: fix regression in references to PEP 3135 implicit __class__ closure variable. Reopens issue #12370, but also updates unittest.mock to workaround that issue
http://hg.python.org/cpython/rev/96ab78ef82a7
msg161704 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-05-27 08:22
As the checkin message says, this is once again a problem on trunk. The relevant test is still in place in test_super.py, I just marked it as an expected failure.

unittest.mock is currently avoiding the problem via the "_safe_super = super" workaround.

So, we need a new patch which fixes the misbehaviour *without* breaking the new tests I just added. I'm thinking something which special cases __class__ references in a ClassBlock to be dual purpose (i.e. both locals *and* closure variables) may be necessary, rather than the current approach of always coercing them to be closure variables.
msg169985 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-09-07 10:49
OK, I think I have a way to fix this that will actually *reduce* the level of special casing needed in the compiler.

Specifically, I think we may be able to make the class statement emit *two* scopes, rather than the current one. The outer scope would be created with MAKE_CLOSURE, and thus names defined there would participate in lexical scoping. The inner scope would use the current MAKE_FUNCTION behaviour, and thus names defined there would be ignored by lexical scoping. "__class__" would then be defined in the outer scope *after* the class has been created. It would roughly like the following, except with __qualname__ still set correctly:

>>> def outer():
...     class inner:
...         def method(self):
...             print(the_class)
...     the_class = inner
...     return inner
...
>>> cls = outer()
>>> cls.method()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: method() missing 1 required positional argument: 'self'
>>> cls().method()
<class '__main__.outer.<locals>.inner'>

The one potential wrinkle I see is whether this might cause a semantic change for name references from the class body to a containing function, but I think the technique is at least worth trying.

If this works, __class__ would just become a completely ordinary cell reference, and override it at class scope should work again. super() itself would still need magic to handle the automatic lookup of the first positional argument to the calling function, but at least the symtable magic should be able to go away.
msg169993 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-09-07 17:00
Wouldn't the following also start working (currently a NameError)?

  class X:
      def f(self):
          print(f.__qualname__)
      def g(self):
          f(None)

  X().f()
  X().g()

How about this[1] (also currently a NameError):

  class Outer:
     class Inner:
         class Worker:
             pass
         class InnerSubclass(Inner):
             class Worker(Inner.Worker):
                 pass

I wouldn't mind the semantic change, but it would be a change nonetheless.


[1] See http://mail.python.org/pipermail/python-list/2011-April/601605.html
msg169994 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-09-07 17:03
Actually, that second would still not work (it would have to pass through the non-lexical inner scope that Nick mentioned).  Is that also the case for the first one?
msg170019 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-09-08 00:40
Yep. The only name in the new scope would be "__class__". Everything else,
including method names, should remain invisible from method bodies. I'm not
100% sure it will work as we want, but that's because I'm not sure if we
can avoid causing a semantic change for classes at module scope. I just
figure it's worth trying.
msg170021 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-09-08 01:46
sounds like it would be worth a shot
msg171830 - (view) Author: Carsten Klein (carsten.klein@axn-software.de) Date: 2012-10-02 20:07
The change was introduced in r30 (Python/symtable.c @ near where it reads /* Special-case super: it counts as a use of __class__ */)
which now enforces that a class that calls super on init will have the correct class information present.

I do not think that this is a bug and that it should be fixed.
Instead it enforces both type safety in respect to classes deriving from a given class hierarchy being forced to report their actual class instead of some fabricated and customly induced one.

If you require such behaviour then you should implement your own meta class that will then override the __class__ property.

And, yes, I do think that Python < 3.0 was wrong in the assumption that one could build up class hierarchies and then break out of that class hierarchy by simply providing a __class__ property that would return a different value as what one would expected.

What do the others think?
msg171832 - (view) Author: Mark Shannon (Mark.Shannon) * Date: 2012-10-02 21:23
There seems to be an ongoing confusion about scopes on this thread.

The __class__ variable used by super() is a non-local variable in the scope of any function using super(), whereas the __class__ used to define the type of an object is a class attribute like any other special attribute e.g. __add__.

The cause of the bug is presumably that the (ast-to-bytecode) compiler fails to differentiate the scopes.

See below for (rather ugly) code which correctly implements the example class presented by Micheal.

class X(object):
        
    @property
    def __class__(self):
        return int
        
class Y
    
    def __init__(self):
        super(X, self).__init__()

X.__init__ = Y.__init__
del Y
        
print (isinstance(X(), int))

>>> X.__init__.__code__.co_freevars[0]
'__class__'

>>> X.__dict__['__class__']
<property object at 0x18f5e68>
msg171873 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-10-03 08:48
Carsten: emulating __class__ is necessary to implement proxy types (and similar utilities like mock objects) correctly. The difference between "x.__class__" is that proxies can remap it to the type of the referent, while "type(x)" will always report the real class of "x" (which may be a proxy like weakref.proxy, or a mock object, as it is in the case Michael is interested in).

Mark: correct, the problem is that the compiler is treating *all* references to __class__ inside a class body as references to the cell variable, when it should really only be doing that for references inside methods - references directly at the class body level should still be to the entry in the class locals namespace that later become attributes of the class object. Hence my idea of introducing a separate closure namespace encapsulating the class namespace to separate the two more cleanly than the current hacky override.
msg176429 - (view) Author: Daniel Urban (daniel.urban) * Date: 2012-11-26 17:20
I tried to implement Nick's idea with the separate scope for __class__. It seems to work, I'm attaching a patch. The patch basically causes the following class statement:

class C(A, B, metaclass=meta):
    def f(self):
        return __class__

To be compiled approximately like this:

def _outer_C(*__args__, **__kw__):
    class _inner_C(*__args__, **__kw__):
        def f(self):
            return __class__
    __class__ = _inner_C
    return _inner_C
C = _outer_C(A, B, metaclass=meta)

It also includes some tests.

(The patch also changes the magic number in Lib/importlib/_bootstrap.py. This caused Python/importlib.h to be regenerated, but I didn't included those changes in the patch, because its a lot, and not very human-readable. Please tell me if I need to include them.)
msg176441 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-11-26 19:11
The ability to close over __args__ and __kw__ in class methods is undesirable.
msg176517 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-11-28 06:43
I agree with Benjamin re: __args__ and __kw__.
msg176520 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-11-28 07:45
From a quick scan of the patch, I suspect the current implementation will also break this code:

  class Outer:
    class InnerParent:
      pass
    class InnerChild(InnerParent):
      pass

The evaluation of the other args to build_class needs to happen before we enter the new outer scope to fix that.

I also agree with the others that this should use hidden variable names for the build_class arguments, rather than __args__ and __kw___.
msg176523 - (view) Author: Daniel Urban (daniel.urban) * Date: 2012-11-28 08:47
Thanks for the review!

Nick, the example with Outer, InnerParent and InnerChild still works (the evaluation happens before we enter the new scope).

Of course you're all right about __args__ and __kw__. I'll try to find a way to hide them.
msg189299 - (view) Author: Roundup Robot (python-dev) Date: 2013-05-15 20:27
New changeset 3d858f1eef54 by Benjamin Peterson in branch 'default':
hide the __class__ closure from the class body (#12370)
http://hg.python.org/cpython/rev/3d858f1eef54
msg189300 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-05-15 20:28
Finally killed this one properly.
History
Date User Action Args
2013-05-15 20:28:08benjamin.petersonsetstatus: open -> closed
resolution: fixed
messages: + msg189300
2013-05-15 20:27:38python-devsetmessages: + msg189299
2012-11-28 08:47:23daniel.urbansetmessages: + msg176523
stage: patch review -> needs patch
2012-11-28 07:45:31ncoghlansetmessages: + msg176520
2012-11-28 06:43:52eric.snowsetmessages: + msg176517
2012-11-26 19:11:29benjamin.petersonsetmessages: + msg176441
2012-11-26 17:20:38daniel.urbansetfiles: + class_super_1.patch
versions: + Python 3.4
messages: + msg176429

components: + Interpreter Core
keywords: + patch, needs review
stage: needs patch -> patch review
2012-10-03 08:48:31ncoghlansetmessages: + msg171873
2012-10-02 21:23:27Mark.Shannonsetmessages: + msg171832
2012-10-02 20:07:20carsten.klein@axn-software.desetnosy: + carsten.klein@axn-software.de
messages: + msg171830
2012-09-08 01:46:48eric.snowsetmessages: + msg170021
2012-09-08 00:40:25ncoghlansetmessages: + msg170019
2012-09-07 17:03:31eric.snowsetmessages: + msg169994
2012-09-07 17:00:34eric.snowsetnosy: + eric.snow
messages: + msg169993
2012-09-07 10:49:07ncoghlansetmessages: + msg169985
2012-07-31 18:42:10cvrebertsetnosy: + cvrebert
2012-05-27 21:47:27Mark.Shannonsetnosy: + Mark.Shannon
2012-05-27 14:00:46meador.ingesetnosy: + meador.inge
2012-05-27 08:22:30ncoghlansetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg161704

stage: committed/rejected -> needs patch
2012-05-27 08:17:22python-devsetmessages: + msg161703
2012-05-20 14:32:34Arfreversetnosy: + Arfrever
2012-05-20 09:27:53daniel.urbansetnosy: + daniel.urban
2011-06-20 14:15:44barrysetmessages: + msg138720
2011-06-20 13:52:19benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg138714
2011-06-20 13:50:22barrysetnosy: + barry
messages: + msg138713
2011-06-20 13:49:07ncoghlansetnosy: + ncoghlan
messages: + msg138712
2011-06-20 00:45:02python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg138677

resolution: fixed
stage: committed/rejected
2011-06-19 22:48:29alexsetnosy: + alex
2011-06-19 22:28:30michael.foordcreate