Skip to content

Conversation

czgdp1807
Copy link
Collaborator

Side notes

While implementing structs I noticed that python_ast_to_asr.cpp is not well implemented. Instead of using recursion and visiting the child nodes, they are handled manually. We should improve upon this later.

@czgdp1807 czgdp1807 added asr ASR related changes llvm LLVM related changes asr_pass ASR pass related changes labels Jun 15, 2022
@czgdp1807 czgdp1807 requested a review from certik June 15, 2022 14:45
@czgdp1807
Copy link
Collaborator Author

@certik This is ready.

@namannimmo10
Copy link
Collaborator

@czgdp1807 -- thanks for adding this. 🎉

# TODO: the above constructor does not initialize `A` in LPython yet, so
# the below does not work:
#assert x.x == 3
#assert x.y == 3.3
Copy link
Contributor

Choose a reason for hiding this comment

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

You can fix this in a subsequent PR, for now I commented it out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will look into this tomorrow.

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

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

Thank you for implementing this!

I pushed in commits that make it work for CPython as well, and commented out asserts that do not work yet. Apparently dataclasses by default require to provide initialization.

@certik certik enabled auto-merge June 15, 2022 17:22
@certik certik merged commit 43dd82b into lcompilers:main Jun 15, 2022
@czgdp1807 czgdp1807 deleted the structs_01 branch June 16, 2022 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
asr_pass ASR pass related changes asr ASR related changes llvm LLVM related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants