Repository navigation
[mypyc] Call subclass __setattr__ for assignments through a base class - #22162
rheard wants to merge 2 commits into
Conversation
Sorry, something went wrong.
| subclasses = class_ir.subclasses() | ||
| # If we can't see all the subclasses, assume that none of them defines it. | ||
| return subclasses is not None and any( | ||
| subclass.has_method("__setattr__") for subclass in subclasses |
There was a problem hiding this comment.
This changes behavior depending on whether a class is compiled using separate compilation or as a single unit, which isn't acceptable. In separate compilation we don't know whether a non-final class has subclasses. Unfortunately, I think this makes the approach unviable.
Sorry, something went wrong.
There was a problem hiding this comment.
This may be a deviation that is too expensive to fix, so we could just document as a known difference in behavior, and possibly eventually provide an opt-in support for the compatible behavior on a per-class basis.
Sorry, something went wrong.
There was a problem hiding this comment.
You're right, I'm not too familiar with separate compilation. I've changed it so that the attribute is only written directly when we know that no subclass can override __setattr__: the class is final, or all subclasses are known and none of them defines it. Otherwise (separate compilation, or allow_interpreted_subclasses=True) the assignment checks tp_setattro at runtime, so both modes now behave the same, and interpreted subclasses with __setattr__ work too. A new multi-module test also runs with separate compilation; it failed with the previous version.
The cost:
- Whole-program builds don't change unless a subclass defines __setattr__ or the class allows interpreted subclasses. For mypy itself, 41 assignments get the check, all in classes with allow_interpreted_subclasses=True (mostly in __init__).
- With separate compilation, every assignment to an attribute of a non-final class gets the check: 3953 in mypy, +2.3% C code. A self-check of mypy compiled with MYPYC_SEPARATE=1 went from 10.50 s to 10.66 s (+1.4%, median of 8 runs). Reads don't change, since attributes are already treated as possibly undefined in separate compilation.
If that's still too much, I can turn this into a documentation change instead.
Sorry, something went wrong.
Fixes mypyc/mypyc#1242
An assignment to an attribute of a native class that has no __setattr__ is compiled to a direct write. The object can be an instance of a subclass that defines __setattr__, and then the method wasn't called:
Now, if a subclass of the class defines __setattr__, the assignment checks whether the type of the object overrides attribute assignment (tp_setattro). If it does, the value is set with PyObject_SetAttr, which calls __setattr__. Otherwise the attribute is written directly as before. Nothing changes for classes that have no subclass with __setattr__.
I used a runtime check instead of always calling PyObject_SetAttr, because that would make every such assignment 10 to 20 times slower for objects that have no __setattr__ (about 2 ns to 26-47 ns per assignment in a loop). With the check, the difference for those objects is under 0.5 ns per assignment.