msg138670 - (view) |
Author: Michael Foord (michael.foord) * |
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) * |
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) * |
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) * |
Date: 2011-06-20 13:52 |
One reason is that it bumps the pyc magic number.
|
msg138720 - (view) |
Author: Barry A. Warsaw (barry) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2012-11-28 06:43 |
I agree with Benjamin re: __args__ and __kw__.
|
msg176520 - (view) |
Author: Nick Coghlan (ncoghlan) * |
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) * |
Date: 2013-05-15 20:28 |
Finally killed this one properly.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:18 | admin | set | github: 56579 |
2013-05-15 20:28:08 | benjamin.peterson | set | status: open -> closed resolution: fixed messages:
+ msg189300
|
2013-05-15 20:27:38 | python-dev | set | messages:
+ msg189299 |
2012-11-28 08:47:23 | daniel.urban | set | messages:
+ msg176523 stage: patch review -> needs patch |
2012-11-28 07:45:31 | ncoghlan | set | messages:
+ msg176520 |
2012-11-28 06:43:52 | eric.snow | set | messages:
+ msg176517 |
2012-11-26 19:11:29 | benjamin.peterson | set | messages:
+ msg176441 |
2012-11-26 17:20:38 | daniel.urban | set | files:
+ 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:31 | ncoghlan | set | messages:
+ msg171873 |
2012-10-02 21:23:27 | Mark.Shannon | set | messages:
+ msg171832 |
2012-10-02 20:07:20 | carsten.klein@axn-software.de | set | nosy:
+ carsten.klein@axn-software.de messages:
+ msg171830
|
2012-09-08 01:46:48 | eric.snow | set | messages:
+ msg170021 |
2012-09-08 00:40:25 | ncoghlan | set | messages:
+ msg170019 |
2012-09-07 17:03:31 | eric.snow | set | messages:
+ msg169994 |
2012-09-07 17:00:34 | eric.snow | set | nosy:
+ eric.snow messages:
+ msg169993
|
2012-09-07 10:49:07 | ncoghlan | set | messages:
+ msg169985 |
2012-07-31 18:42:10 | cvrebert | set | nosy:
+ cvrebert
|
2012-05-27 21:47:27 | Mark.Shannon | set | nosy:
+ Mark.Shannon
|
2012-05-27 14:00:46 | meador.inge | set | nosy:
+ meador.inge
|
2012-05-27 08:22:30 | ncoghlan | set | status: closed -> open resolution: fixed -> (no value) messages:
+ msg161704
stage: resolved -> needs patch |
2012-05-27 08:17:22 | python-dev | set | messages:
+ msg161703 |
2012-05-20 14:32:34 | Arfrever | set | nosy:
+ Arfrever
|
2012-05-20 09:27:53 | daniel.urban | set | nosy:
+ daniel.urban
|
2011-06-20 14:15:44 | barry | set | messages:
+ msg138720 |
2011-06-20 13:52:19 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages:
+ msg138714
|
2011-06-20 13:50:22 | barry | set | nosy:
+ barry messages:
+ msg138713
|
2011-06-20 13:49:07 | ncoghlan | set | nosy:
+ ncoghlan messages:
+ msg138712
|
2011-06-20 00:45:02 | python-dev | set | status: open -> closed
nosy:
+ python-dev messages:
+ msg138677
resolution: fixed stage: resolved |
2011-06-19 22:48:29 | alex | set | nosy:
+ alex
|
2011-06-19 22:28:30 | michael.foord | create | |