-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[mypyc] Support attributes that override properties #14377
Conversation
@@ -223,8 +236,68 @@ def prepare_class_def( | |||
# Supports copy.copy and pickle (including subclasses) | |||
ir._serializable = True | |||
|
|||
# We sort the table for determinism here on Python 3.5 | |||
for name, node in sorted(info.names.items()): | |||
# Check for subclassing from builtin types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to restructure this to do things in a different order. Most of the logic hasn't changed. I also extracted some functionality to separate methods since the function was already quite big.
This comment has been minimized.
This comment has been minimized.
Is the eventual goal to allow this for mypyc? (Which throws an exception in CPython) class B:
@property
def x(self) -> int:
return 0
class C(B):
x: int
def __init__(self) -> None:
self.x = 4 |
Generally the goal is to reject things that don't work in CPython. Updated the example to something that actually works in CPython: class B:
@property
def x(self) -> int:
return 0
class C(B):
x: int = 0 # Attribute overrides a property Added #14413 to track the mypy false negative when there is no initializer. I'll also need to remove cases that aren't supported by CPython from the tests in added in this PR. |
I always thought that the idiomatic way to override a read-only property with a writable attribute was supposed to be this, but mypy unfortunately emits a false positive for this snippet: class A:
_x: int
@property
def x(self) -> int:
return self._x
class B(A):
@A.x.setter # mypy complains with 'error: "Callable[[A], int]" has no attribute "setter" [attr-defined]'
def x(self, val: int) -> None:
self._x = val
b = B()
b.x = 42
b.x (This false positive is already tracked in #5936, and #1465 (comment), and, until I closed it a few minutes ago, #4644.) |
To override a property with an attribute, an initializer is needed.
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
This idiom is almost nonexistent in the codebases I work on, though mypy not supporting it may be the reason why developers use something different. Still, it would be nice to support it. I focused on the particular use case addressed in this PR since I have a follow-up PR that needs this. |
Code like this is now supported by mypyc:
The implementation generates implicit getter/setter methods for attributes as needed and puts them in the vtable. I had to change both the irbuild "prepare" pass (where we generate declarations), irbuild main pass (where we generate implicit accessor IRs), and codegen (where implicit properties aren't visible externally to CPython).
Also fix a minor mypy bug related to overriding properties and multiple inheritance that I encountered.
This doesn't handle glue methods yet.