TypeError when using super() and slots=True

[p-himik]

This code

import attr


@attr.s(slots=True)
class A:
    def f(self):
        pass


@attr.s(slots=True)
class B(A):
    def f(self):
        super().f()


B().f()

results in this error

Traceback (most recent call last):
  File "test.py", line 16, in <module>
    B().f()
  File "test.py", line 13, in f
    super().f()
TypeError: super(type, obj): obj must be an instance or subtype of type

If I use super(B, self), everything works just fine.

[Tinche]

Huh, interesting. Slots are a little tricky since we can't "add" slots to a class that's already defined, so we switch the class with another one in attr.s. This is what's probably causing the problem.

I need to figure out how to debug this :)

[p-himik]

I guess that some other more intricate workflow involving isinstance will be broken in this case too.
May'be it's possible to switch the parent class with the updated one?

[Tinche]

Reading up on it a little (https://www.python.org/dev/peps/pep-3135/), there will probably be problems with any function inside the class using __class__ or no-arg super(). (Have to admit I wasn't even aware the __class__ name was available inside of methods.)

Unfortunately these function cells that contain the reference to the enclosing class appear to be immutable.

Anything outside the class should be fine.

At the very least, it will be possible to raise an informative error at class definition time.

More research needed.

[moshez]

Just my obsession with minimal example: notice that this reproduces even in this simpler example:

import attr

@attr.s(slots=True)
class B:
    def f(self):
        super()

B().f()

[hynek]

Something tells me that this might be unsolvable unless there’s some secret backdoor we can use. :|

[moshez]

Perhaps the only way is to error out if any of the functions inside the class refer to "super" or "class" with an explanation.

[perkinslr]

Could do it by recreating the function objects, the function cells are immutable, but creating a duplicate function object which contains a cell pointing to the new class is possible. Unfortunately, explicitly creating cell instances from python (à la types.FunctionType()) is not possible, so it would require including a stub method in the new type that references the cell and then creating a new function using the old parameters except for swapping the closure.

It's also possible via the python C API to set the cell contents, which may be the simplest solution. I've tested it a bit and it seems to work.

class A(object):
    def printClass(self):
        print(__class__)

class NewA(object):
    pass

import ctypes
libpython = ctypes.cdll.LoadLibrary('/usr/lib64/libpython3.5.so')
libpython.PyCell_Set(ctypes.py_object(A.printClass.__closure__[0]), ctypes.py_object(NewA))

A().printClass()

It would require figuring out if the cell holds a weakref or a normal reference to the cell contents, so as to not screw up the reference count.

[hynek]

I wonder if we couldn’t fix this using __new__ once we drop the requirement, that attrs definitions are replaced by Attribute instances (since class attributes on __slots__ classes are immutable).

[p-himik]

Just got bitten by it again - only this time while using a metaclass.

But we can add __slots__ in a metaclass and according to https://stackoverflow.com/questions/11091609/setting-a-class-metaclass-using-a-decorator you can add a metaclass by using a decorator.
So what if @attr.s(...) instead of directly altering class attributes and/or the class itself, would just add a metaclass, taking already provided metaclass into account?

[p-himik]

Hmm, this method still doesn't make TypeError go away, although the same error doesn't happen when using metaclass kwarg when creating a class.
Probably it's possible to create a metaclass that would be used in @attr.s(...) when a user doesn't need super() and used as a regular metaclass when there's a need to super().

[hynek]

Yay, I just ran into the problem too. :|

Currently we have problems on both sides of the fence:

• If we replace the class, super() stops working
• If we don’t, a bunch of other assumptions don’t work like Add a more idiomatic way to define hashing #202

Looks like a lose-lose scenario. :|

[Tinche]

There's a way to fix this on CPython.

Content warning: hairy code ahead ;)

Simplest reproducer:

import attr


@attr.s(slots=True)
class C:
    a = attr.ib()

    def t(self):
        return __class__


>>> print(C(1).t() is C)
False

The problem is this:

>>> C.t.__closure__[0].cell_contents
<class '__main__.C'>   # This is not our class, but the old class.

The fix:

import attr


@attr.s(slots=True)
class C:
    a = attr.ib()

    def t(self):
        return __class__

import ctypes
PyCell_Set = ctypes.pythonapi.PyCell_Set
PyCell_Set.argtypes = (ctypes.py_object, ctypes.py_object)
PyCell_Set.restype = ctypes.c_int
PyCell_Set(C.t.__closure__[0], C)

>>> print(C(1).t() is C)
True

We need to ask our PyPy friends how to handle this on PyPy, though.

[hynek]

That’s…disgusting. :D

I’d know whom to ask however maybe we should discuss what the canonical answer in attrs should be: always new class (like slots=True, requires interpreter internals) or patched class (classic, requires potentially meta classes to be done properly—I intend to look at a __new__-based approach once we’re rid of .Attribute access)?

Yesterday I’d have said “always new class” but the repercussions seem worse than patching around.

[Tinche]

I don't actually know how a metaclass approach would work. Could you show me a prototype just for me to get the gist?

[hynek]

Haven’t tried anything yet but you can set __slots__ in __new__. I hope to remember, that the reason why we can’t do it, is that currently the slots are filled and thus immutable.

[Tinche]

Alright, got a response from the PyPy team (Armin Rigo of course :)

It's actually easier there:

C.t.__closure__[0].__setstate__((C,))

[hynek]

Fixed by #226

[radugrosu]

This still happens on 19.3.0, for propertys.

[wsanchez]

@radugrosu Would you mind opening a new ticket with an example?

[radugrosu]

@radugrosu Would you mind opening a new ticket with an example?

Done.