super() broken with classmethods

[afcdb1a8-bc5d-4852-a2a5-71b4ae6361ae]

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: ```python assignee = 'https://github.com/gvanrossum' closed_at = created_at = labels = ['interpreter-core'] title = 'super() broken with classmethods' updated_at = user = 'https://github.com/pjeby' ``` bugs.python.org fields: ```python activity = actor = 'gvanrossum' assignee = 'gvanrossum' closed = True closed_date = None closer = None components = ['Interpreter Core'] creation = 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'] ```

[afcdb1a8-bc5d-4852-a2a5-71b4ae6361ae]

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);

[61337411-43fc-4a9c-b8d5-4060aede66d0]

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.

[afcdb1a8-bc5d-4852-a2a5-71b4ae6361ae]

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 imself 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.

[afcdb1a8-bc5d-4852-a2a5-71b4ae6361ae]

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.

[afcdb1a8-bc5d-4852-a2a5-71b4ae6361ae]

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.

[afcdb1a8-bc5d-4852-a2a5-71b4ae6361ae]

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.