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

Add support for class members #2821

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xaerru
Copy link

@xaerru xaerru commented Jan 24, 2025

Fix #2723

Add support for class members by identifying them by the new intent ClassMember and creating a global variable for each class member.

All the tests are not passing yet, but I need a review to know if I'm going in the right direction.

@xaerru xaerru marked this pull request as ready for review January 25, 2025 17:55
@xaerru xaerru force-pushed the fix-direct-class-member-access branch from 8c935e7 to cb58cda Compare January 29, 2025 13:50
@xaerru
Copy link
Author

xaerru commented Jan 30, 2025

The tests are now passing.

I now need to implement functionality for when an instance variable accesses a class member. If it performs a read, it should access the global variable. But, if it writes to the class member, a copy should be made for that specific instance and used for any further reads and writes.

Currently, instance variables only access the global class member.

class A:
    c: i32 = 10
    def __init__(self:"A") -> None:
        self.d: i32 = 20

a: A = A()
a.c = 30 # Makes a copy
print(a.c) # 30
print(A.c) # Should be 10, it is currently 30

I wanted advice on how to implement this and also if the current design of using global variables for class members is good or not.

One way can be adding the class members definitions to __init__'s body. But then we need to keep track if a copy has been created or not.

@@ -159,7 +159,7 @@ expr
| ArrayBroadcast(expr array, expr shape, ttype type, expr? value)
| BitCast(expr source, expr mold, expr? size, ttype type, expr? value)
| StructInstanceMember(expr v, symbol m, ttype type, expr? value)
| StructStaticMember(expr v, symbol m, ttype type, expr? value)
| StructStaticMember(identifier class_name, symbol m, ttype type, expr? value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original is correct, we should not use strings, but rather links to a symbol table.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted.

It was also not being used. The global name is given by the member symbol's name.

@xaerru xaerru force-pushed the fix-direct-class-member-access branch from cb58cda to d6e350e Compare January 31, 2025 18:45
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.

Direct class member access doesn't work
2 participants