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

[mypyc] Support attributes that override properties #14377

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Jan 2, 2023

Code like this is now supported by mypyc:

class B:
    @property
    def x(self) -> int:
        return 0

class C(B):
    x: int  # Attribute overrides a property

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.

@@ -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
Copy link
Collaborator Author

@JukkaL JukkaL Jan 2, 2023

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.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2023

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant