Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Argumentless super() fails in classes constructed with type() #74130

Open
mr-nfamous mannequin opened this issue Mar 30, 2017 · 8 comments
Open

Argumentless super() fails in classes constructed with type() #74130

mr-nfamous mannequin opened this issue Mar 30, 2017 · 8 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@mr-nfamous
Copy link
Mannequin

mr-nfamous mannequin commented Mar 30, 2017

BPO 29944
Nosy @ncoghlan, @vstinner, @benjaminp, @vadmium, @MojoVampire, @zhangyangyu, @DimitrisJim, @mr-nfamous, @cdonovick

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 = None
closed_at = None
created_at = <Date 2017-03-30.06:36:49.869>
labels = ['type-bug']
title = 'Argumentless super() fails in classes constructed with type()'
updated_at = <Date 2022-03-28.16:25:17.773>
user = 'https://github.com/mr-nfamous'

bugs.python.org fields:

activity = <Date 2022-03-28.16:25:17.773>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2017-03-30.06:36:49.869>
creator = 'bup'
dependencies = []
files = []
hgrepos = []
issue_num = 29944
keywords = []
message_count = 8.0
messages = ['290823', '290843', '290856', '290858', '290889', '291347', '291349', '416185']
nosy_count = 9.0
nosy_names = ['ncoghlan', 'vstinner', 'benjamin.peterson', 'martin.panter', 'josh.r', 'xiang.zhang', 'Jim Fasarakis-Hilliard', 'bup', 'donovick']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29944'
versions = ['Python 3.6']

@mr-nfamous
Copy link
Mannequin Author

mr-nfamous mannequin commented Mar 30, 2017

The simplest example:

def mydec(cls):
    return type(cls.__name__, cls.__bases__, dict(cls.__dict__))

@mydec
class MyList(list):
    
    def extend(self, item):
        super(MyList, self).extend(item)
        
    def insert(self, index, object):
        super().insert(index, object)
>>> lst = MyList()
>>> lst.extend([2,3])
>>> lst.insert(0, 1)
TypeError: super(type, obj): obj must be an instance or subtype of type
>>> lst
[2, 3]

If this is intended behavior, at least the error message could be fixed.

@mr-nfamous mr-nfamous mannequin added the type-bug An unexpected behavior, bug, or error label Mar 30, 2017
@ncoghlan
Copy link
Contributor

Interestingly, even types.new_class misbehaves in this case:

>>> def mydec(cls):
...     return types.new_class(cls.__name__, cls.__bases__, exec_body=lambda ns: ns.update(cls.__dict__))
... 
>>> @mydec
... class MyList(list):
...     def insert(self, idx, obj):
...         super().insert(idx, obj)
... 
>>> MyList().insert(0, 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 4, in insert
TypeError: super(type, obj): obj must be an instance or subtype of type

The error message is confusing here because it's using 'type' as a metavariable to refer to the first argument of super(), *not* to the builtin called "type".

If we poke around in the MyList.insert closure, we can see the origin of the problem:

>>> MyList.insert.__closure__[0].cell_contents
<class '__main__.MyList'>
>>> MyList.insert.__closure__[0].cell_contents is MyList
False

The class cell is still bound to the originally created class object, but the decorator threw that away and returned a completely different object. As a result, the "self" passed to the bound method is *not* an instance of the type stored in the "__class__" cell, and super() complains.

Given that super() doesn't take keyword arguments, perhaps we should propose on python-dev that "type" be changed to "starting_type" in the super() error messages and documentation?

@MojoVampire
Copy link
Mannequin

MojoVampire mannequin commented Mar 30, 2017

It looks like this is a general problem caused by the fact that a function that is:

  1. Defined in a class
  2. References the name "super" (even if it's a local variable name, even if it's never called)

isn't "just" a plain function. Even though Python 3 officially removed the concept of unbound methods, there is still some unbound method-like behavior going on, which makes that sort of function intrinsically tied to where it executes.

Specifically, a function of that sort is effectively a closure, where the closure scope defines __class__ (insert.__code__.co_freevars will be non-empty with the string '__class__' in it); the value of that closure variable cell is set when the class finishes being defined (that is, the closure cell is empty when the def block finishes, but when the class block itself completes, the cell is populated with a reference to the class).

No argument super (specifically, super_init) relies on this assistance, as it looks up the __class__ cell in the caller's frame when not provided a class explicitly. In your repro, the problem is that __class__ is set to the original MyList (it was set at the moment the class MyList(list): block finished, before the decorator was invoked), but at call time, the self instance is of the new type you created (which has the same name, but the name isn't looked up, it has a cached reference to the class).

The same problem applies if you do:

class MyList(list):
    def insert(self, index, object):
        super().insert(index, object)

class MyOtherList(list):
    insert = MyList.insert

because MyList.insert is permanently tied to MyList, it doesn't get "reclosured" as a result of MyOtherList using it. Similarly:

class MyList(list):
    pass

def insert(self, index, object):
    super().insert(index, object)

MyList.insert = insert

fails when insert is called with error: "RuntimeError: super(): __class__ cell not found", because functions def-ed outside a class block entirely don't have __class__ as "virtual closure" scope.

It's arguable whether this should be changed: If you're using super() without arguments, the implicit behavior is that it works with "whatever class I'm currently defining", so trying to reuse it with some other class is trying to extend that implicit behavior to "whatever class I was eventually assigned to". Making it work would mean that already "class bound" methods must be rebound when assigned to a new class, and non-bound methods must be bound (in both cases, you'd need to handle this when implicitly assigned in the class body, or explicitly assigned later as an attribute on the constructed class).

Doing so would break at least one pattern someone might be using already, where they have multiple inheritance, and while most methods should prioritize the first class in the MRO, they want a specific method to bypass one or more classes in the MRO without calling it. For example:

class Stuff:  # Top of diamond, just so super().stuff() valid in all children
    def stuff(self):
        print("Stuff")

class Spam:
    def stuff(self):
        print("Spam")
        super().stuff()

class Eggs:
    def stuff(self):
        print("Eggs")
        super().stuff()

class SpamAndEggsWithoutSpam(Spam, Eggs):
    stuff = Eggs.stuff  # I'd like my stuff without Spam

Under the current design, Eggs.stuff is already bound to Eggs, so this works; you see:

Eggs
Stuff

printed, but not Spam. If rebinding were made a thing, you'd end up with:

Eggs
Spam
Eggs
Stuff

with the first Eggs being from the rebound method.

@MojoVampire
Copy link
Mannequin

MojoVampire mannequin commented Mar 30, 2017

This is what I get for leaving a response up and working on it intermittently for three hours, and not rechecking: I found the same basic problem.

For the record, I also found a terribly hacky way of doing this sort of rebinding manually, should you actually need to do so under the current design:

def make_class_closure(__class__):
    '''Makes a one-tuple closure with a cell for the provided class'''
    return (lambda: super).__closure__

class MyList(list):
    def insert(self, index, object):
        super().insert(index, object)

class MyList2(list):
    pass

MyList2.insert = types.FunctionType(MyList.insert.__code__, globals(), closure=make_class_closure(MyList2))

It would need further improvements to recognize when it needs to run, deal with existing closures, handle top-level functions without __class__ in their closure co_freevars, etc., and I won't even pretend that's a sane workaround for the problem, but it does demonstrate what is happening/what would need to be changed to make it work, if indeed that's desirable.

@ncoghlan
Copy link
Contributor

Right, there's a very similar instance-method-reuse related problem described in http://bugs.python.org/issue29270, where ctypes re-uses a class namespace is define a swapped-endianness version of the originally defined class.

The easiest place to deflect conflicts is at the point where bound instance methods are created:

>>> bound_method = MyList().insert
>>> bound_method.__self__.__class__
<class '__main__.MyList'>
>>> bound_method.__func__.__closure__[0].cell_contents
<class '__main__.MyList'>
>>> bound_method.__self__.__class__ is bound_method.__func__.__closure__[0].cell_contents
False

However, that method of detection only works for plain instance methods that only close over the __class__ variable: as soon as you wrap them in a decorator, you may not have easy access to the __closure__ attribute any more, and if the method has closure references to more than just __class__, then it's a bit more work to find the right closure index from outside the function.

@terryjreedy terryjreedy changed the title Argumentless super() calls do not work in classes constructed with type() Argumentless super() fails in classes constructed with type() Apr 1, 2017
@vadmium
Copy link
Member

vadmium commented Apr 9, 2017

In bpo-23674, I posted a patch that changes to consistent parameter names (subclass, self). The exception message would avoid “type”, becoming

super(subclass, self): self must be an instance or subtype of subclass

@mr-nfamous
Copy link
Mannequin Author

mr-nfamous mannequin commented Apr 9, 2017

Just wanted to add that I found this when I was trying to find a way to decorate all methods in a class without using a metaclass or modifying __getattr__.

Using Josh's workaround, here's a simple demonstration that (at least at first glance) prints a message when a method is called and when it is finished:

def mydec(*, enter=True, exit=True):
    def make_closure(__class__):
        return (lambda: super).__closure__
    def outer(cls):
        def wrapper(method):
            @functools.wraps(method)
            def inner(*args, **kwargs):
                if enter:
                    print('entering', method.__qualname__)
                r = method(*args, **kwargs)
                if exit:
                    print('exiting', method.__qualname__)
                return method(*args, **kwargs)
            return inner    
        for name, value in cls.__dict__.items():
            if isinstance(value, types.FunctionType):
                method = types.FunctionType(getattr(cls, name).__code__,
                                            globals(),
                                            closure=make_closure(cls))
                setattr(cls, name, wrapper(method))
        return cls
    return outer

@vstinner
Copy link
Member

See also bpo-47143 "Add functools.copy_class() which updates closures".

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants