Whoa. Looking at the diff I realized for the first time that the two subclasses, TypeVar and ParamSpec, have a __dict__ slot, so adding slots to the base class does not actually make a difference.

I'm sure that the __dict__ slots exist for a reason -- if we removed those, some user code would break that assigns additional attributes.

Now I wonder if the problem isn't the inverse -- why do the two subclasses have slots? What does that add?

@gvanrossum I can imagine the __slots__ still serve their purpose of improved attribute lookup speed 🤔 , although I of course don't know the reasoning when the code was written.

I can imagine __dict__ was added in the end to enable class-like attributes (e.g. __module__, __doc__, etc.) to be set in user code. (Having __dict__ in __slots__ is explicitly allowed for this possibility.)

Possible solutions:

1. Keep as many slots as can be justified

   • keep the slots in the base class (for improved attribute lookup speed)
   • remove __slots__ = ('__dict__', ) from ParamSpec. It serves no use.
   • keep __slots__ in TypeVar (for __name__ lookup speed I suppose)

2. Remove slots from all three classes. Benefits are marginal and some other typing classes (like _GenericAlias) don't seem to declare them either.

Interesting. I admit that when I implemented ParamSpec, it was meant to mirror any existing behavior in TypeVar, and I didn't give much thought into whether it required a __dict__.

Digging through commit history tells me that we only added a __dict__ to TypeVar recently (3.9-ish) c1c7d8e. Prior to this, TypeVar had __slots__ but no __dict__, all the way back to when typing was still in a separate package from Python.

@sobolevn I'm inclined to agree, although adding __doc__ to the allowed slots may be a good idea.

I'd be very reluctant to remove the __dict__ slot -- even though it's undocumented and we don't test for it, someone is likely using it (since having a __dict__ is the default for most classes). I don't believe that attribute lookup speed is ever a factor for these.

I suppose we could ask @serhiy-storchaka if he recalls why he added the __dict__ slot in #19719.

It's possible that there's a space-saving reason for having __slots__ that include a __dict__ -- I'm guessing that as long as you don't use the __dict__ it's NULL, so for the common case (where all attributes fit in slots) the __slots__ version is more space-efficient.

I used to just know this kind of stuff, but the code is a lot more convoluted now due to key-sharing (the dict keys are stored, once, on the class object, and the instances only have a values array), and is about to become more so -- for 3.11 Mark Shannon has improvements that store the values array in a more compact form (IIRC without holes).

Anyway, I think the sensible solution is to remove the __slots__ from these classes.

@ariebovenberg is right, __dict__ is needed for supporting the __module__ attribute in instances. You cannot add '__module__' in __slots__, because descriptor __module__ conflicts with the same-named class attribute.

'__dict__' was explicitly added in __slots__ in c1c7d8e. Previously there was an implicit __dict__ inherited from _Immortal which lacked __slots__. After adding an empty __slots__ in _Immortal I was needed to add '__dict__' in __slots__ for TypeVar. Alternate solution would be to remove __slots__ in TypeVar, but I hoped to find other solution of this conflict sometime.

There was no conflict for __name__, because __name__ of class is accessed via a metaclass descriptor. But __module__ of class is saved in class' dict. Adding a dedicated field in PyTypeObject and descriptor in class type would solve this conflict. It is a relatively large change which breaks binary compatibility of PyTypeObject (which is not in the stable API anyway) and may simplify and speed up access to __module__ attribute of classes. But there may be side effects.

@serhiy-storchaka What do you think of just getting rid of the slots for TypeVar and ParamSpec?

What do you think of just getting rid of the slots for TypeVar and ParamSpec?

I am fine with this.

@serhiy-storchaka @gvanrossum about the slots on _TypeVarLike: I assume we don't keep them there either, since we can assume it'll only be subclassed by ParamSpec and TypeVar?

If its purpose would be a general mixin, __slots__ would make more sense.

@serhiy-storchaka @gvanrossum about the slots on _TypeVarLike: I assume we don't keep them there either, since we can assume it'll only be subclassed by ParamSpec and TypeVar?

Indeed.

If its purpose would be a general mixin, __slots__ would make more sense.

It is not.

@Fidget-Spinner Do you want to land this? Make sure to edit the commit title and message so they match what's ultimately done.

@Fidget-Spinner: Please replace # with GH- in the commit message next time. Thanks!

@gvanrossum Sorry, I didn't realize that GH mobile app's squash and merge doesn't allow me to edit the message. After pressing the button it just merged it without prompting me for anything 😞.

Lesson learnt: always do the major things on the computer. I'll revert and redo the message later today.

Anyways, @ariebovenberg thanks for the patch, and congrats on your first contribution to CPython!

@gvanrossum Sorry, I didn't realize that GH mobile app's squash and merge doesn't allow me to edit the message. After pressing the button it just merged it without prompting me for anything 😞.

Their mobile website allows it though. :-)

Lesson learnt: always do the major things on the computer. I'll revert and redo the message later today.

I wouldn't bother. The summary line is correct: "Remove slots from typing.TypeVar, .ParamSpec ". The commit body is spammy, but that doesn't matter enough to revert and fix the commit. We can live with this!

Anyways, @ariebovenberg thanks for the patch, and congrats on your first contribution to CPython!

Indeed.