super() broken with classmethods

[pjeby]

BPO	535444
Nosy	@mwhudson, @gvanrossum, @loewis, @pjeby

^{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:

assignee = 'https://github.com/gvanrossum'
closed_at = <Date 2002-04-02.21:27:21.000>
created_at = <Date 2002-03-26.22:13:29.000>
labels = ['interpreter-core']
title = 'super() broken with classmethods'
updated_at = <Date 2002-04-02.21:27:21.000>
user = 'https://github.com/pjeby'

bugs.python.org fields:

activity = <Date 2002-04-02.21:27:21.000>
actor = 'gvanrossum'
assignee = 'gvanrossum'
closed = True
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2002-03-26.22:13:29.000>
creator = 'pje'
dependencies = []
files = []
hgrepos = []
issue_num = 535444
keywords = []
message_count = 10.0
messages = ['9995', '9996', '9997', '9998', '9999', '10000', '10001', '10002', '10003', '10004']
nosy_count = 4.0
nosy_names = ['mwh', 'gvanrossum', 'loewis', 'pje']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue535444'
versions = ['Python 2.2']

[pjeby]

Using super() in a classmethod breaks in Python 2.2.
Apparently, when super looks up an attribute from the
__mro__ sequence, it calls the found descriptor's
__get__ with the descriptor itself as the 'type'
argument, which breaks horribly with class methods
(which always binds to the type argument).

Presumably, the fix is to pass a NULL type argument,
which should work with the recent fixes to the
classmethod's __get__ logic. In other words, this code
in the super_getattro function Objects/typeobject.c:

tmp = f(res, su->obj, res);

should probably actually read:

tmp = f(res, su->obj, NULL);

[loewis]

Logged In: YES
user_id=21627

Can you give an example of how to break it? Please also
report what your example does when you run it.

[pjeby]

Logged In: YES
user_id=56214

class cm1(object):
    def cmm(klass):
        print klass
    cmm = classmethod(cmm)

class cm2(cm1):
    def cmm(klass):
        super(cm2,klass).cmm()
    cmm = classmethod(cmm)

cm2.cmm()

The above code prints "<classmethod object at 0x00A9B930>",
demonstrating that super(cm2,klass).cmm is bound improperly.
(It should print <class '__main__.cm2'>, analagous to how
calling cm1.cmm() directly prints <class '__main__.cm1'>.)
You can more specifically verify this like so:

>>> cm1.cmm.im_self
<class '__main__.cm1'>
>>> cm2.cmm.im_self
<class '__main__.cm2'>
>>> super(cm2,cm2).cmm.im_self
<classmethod object at 0x00A9B930>
>>> 

The last item's im_self should of course be <class
'__main__.cm2'>. As I said, the problem is that
super_getattro incorrectly asks the classmethod descriptor
to bind to *itself*, rather than to a type.

Note that if you use the pure Python example version of
"super" defined in the python.org/2.2/descrintro.html
document, the above examples work correctly as long as you
use a version of Python that has the "classmethod core dump"
problem fixed. However, the builtin super() never works
correctly for classmethods, whether the "classmethod core
dump" is fixed or not.

[pjeby]

Logged In: YES
user_id=56214

Ugh. I just realized that my "presumable fix" is actually
wrong. I checked back on my "Python super" workaround, and
realized I modified Guido's example slightly, to call
__get__(self.__obj__,starttype), instead of
__get__(self.__obj__). This implies that the fix to
super_getattro is a little more complicated, since
super_getattro doesn't have a C variable equivalent to
starttype in the Python version of super. :(

[mwhudson]

Logged In: YES
user_id=6656

Unless someone can come up with a obviously correct patch
(and convince Guido that it's obviously correct) very soon,
this isn't going to go into 2.2.1.

[pjeby]

Logged In: YES
user_id=56214

Patch bpo-537536 submitted to fix this. "make test" output is
same as before the patch. If someone can give me an idea of
where/how to insert a regression test for the bug being
fixed, I'll submit a patch for that, too. Thanks.

[mwhudson]

Logged In: YES
user_id=6656

Assign to Guido.

Bearing in mind that I haven't even tried to understand this
bug, the fact that you can't come up with a test case stands
against you... if you're just asking where to put it, stick
it in test_descr.

[pjeby]

Logged In: YES
user_id=56214

I was indeed just asking where to put it, and how to
*insert* the test. Anyway, I found that most of what was
needed for the test was already in
test_descr.classmethods(), there were just a few conditions
using super() that needed adding. I've uploaded the patch
for test_descr to the patch bpo-537536 for this bug.

By the way, the bug/patch submission guidelines were a
little unclear to me; specifically whether I was supposed to
put the patch with the bug or the bug with the patch or
upload everything to both or what. Hope my ignorance hasn't
inconvenienced anyone; this is my first time submitting
Python bugs and fixes. Also, I hadn't worked with the test
framework used in Python's test suite before, although I've
done quite a bit with unittest in my own code.

[mwhudson]

Logged In: YES
user_id=6656

OK, you've found the right place, good.

The bug/patch guidlines are probably confusing because,
unless you're part of the Python project I think you can't
attach a file to a report you didn't submit. Generally I
prefer patches to be attached to bugs, but that's just my
opinion, I don't know what other developers think. I also
think the whole bug/patch division is misguided, but that's
another rant entirely.

I think you're doing fine! Now we just wait for Guido to
stop changing nappies :)

[gvanrossum]

Logged In: YES
user_id=6380

I should probably mention that I checked in Phillip's patch
537536, closing this issue.