Skip to content

Initial implementation of Inheritance and Polymorphic functions#2801

Merged
Thirumalai-Shaktivel merged 2 commits intolcompilers:mainfrom
tanay-man:inheritance
Aug 18, 2024
Merged

Initial implementation of Inheritance and Polymorphic functions#2801
Thirumalai-Shaktivel merged 2 commits intolcompilers:mainfrom
tanay-man:inheritance

Conversation

@tanay-man
Copy link
Copy Markdown
Contributor

@tanay-man tanay-man commented Aug 12, 2024

No description provided.

certik
certik approved these changes Aug 12, 2024
View reviewed changes
Copy link
Copy Markdown
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.

I think this is good. @Thirumalai-Shaktivel if you could also review it, that would be great.

@certik
Copy link
Copy Markdown
Contributor

certik commented Aug 12, 2024

Not sure why the kernel is failing:

[doctest] doctest version is "2.4.8"
[doctest] run with "--help" for options
0 0 0 0 0 0 0 0 0 0 �
test_lpython: /home/runner/micromamba/envs/lp/include/llvm/IR/DataLayout.h:636: uint64_t llvm::StructLayout::getElementOffset(unsigned int) const: Assertion `Idx < NumElements && "Invalid element idx!"' failed.
===============================================================================
/home/runner/work/lpython/lpython/src/lpython/tests/test_llvm.cpp:1556:
TEST CASE:  PythonCompiler classes

/home/runner/work/lpython/lpython/src/lpython/tests/test_llvm.cpp:1556: FATAL ERROR: test case CRASHED: SIGABRT - Abort (abnormal termination) signal

It looks like it could be caused by this PR.

Thirumalai-Shaktivel
Thirumalai-Shaktivel reviewed Aug 13, 2024
View reviewed changes
Comment thread integration_tests/class_05.py Outdated
Thirumalai-Shaktivel
Thirumalai-Shaktivel reviewed Aug 13, 2024
View reviewed changes
Comment thread src/lpython/semantics/python_ast_to_asr.cpp Outdated
@Thirumalai-Shaktivel
Copy link
Copy Markdown
Collaborator

Thirumalai-Shaktivel commented Aug 13, 2024 •
edited
Loading

I think the kernel failure is not related to this PR.

Edit: It seems related. I will check and report back

@tanay-man
Copy link
Copy Markdown
Contributor Author

tanay-man commented Aug 16, 2024

We have to merge this with #2795 before merging as some of the ASR-pass related modifications are present there.

Thirumalai-Shaktivel
Thirumalai-Shaktivel reviewed Aug 16, 2024
View reviewed changes
Comment thread integration_tests/class_06.py Outdated
@tanay-man tanay-man mentioned this pull request Aug 16, 2024
Open
@Thirumalai-Shaktivel
Copy link
Copy Markdown
Collaborator

Thirumalai-Shaktivel commented Aug 18, 2024

Can you please rebase the branch with main?

Thirumalai-Shaktivel
Thirumalai-Shaktivel reviewed Aug 18, 2024
View reviewed changes
Comment thread src/lpython/semantics/python_ast_to_asr.cpp Outdated
Thirumalai-Shaktivel
Thirumalai-Shaktivel reviewed Aug 18, 2024
View reviewed changes
Comment thread src/lpython/semantics/python_ast_to_asr.cpp
Thirumalai-Shaktivel
Thirumalai-Shaktivel reviewed Aug 18, 2024
View reviewed changes
Comment thread src/lpython/semantics/python_ast_to_asr.cpp Outdated
Thirumalai-Shaktivel
Thirumalai-Shaktivel reviewed Aug 18, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Thirumalai-Shaktivel Thirumalai-Shaktivel left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. It looks great!

I left minor suggestions

@Thirumalai-Shaktivel
Copy link
Copy Markdown
Collaborator

Thirumalai-Shaktivel commented Aug 18, 2024

Update or rebase this PR and it is good to merge.

Thirumalai-Shaktivel
Thirumalai-Shaktivel reviewed Aug 18, 2024
View reviewed changes
Comment thread src/libasr/pass/pass_utils.cpp Outdated
@Thirumalai-Shaktivel
Copy link
Copy Markdown
Collaborator

Thirumalai-Shaktivel commented Aug 18, 2024

Also, change the PR heading to appropriate description

@Thirumalai-Shaktivel
Copy link
Copy Markdown
Collaborator

Thirumalai-Shaktivel commented Aug 18, 2024

Also, let's clean up the commit messages.

@Thirumalai-Shaktivel
Copy link
Copy Markdown
Collaborator

Thirumalai-Shaktivel commented Aug 18, 2024 •
edited
Loading

See: #783 (comment), #319 (comment)

@tanay-man
Copy link
Copy Markdown
Contributor Author

tanay-man commented Aug 18, 2024

Do I have to remove all the commits in the history?

@tanay-man
Copy link
Copy Markdown
Contributor Author

tanay-man commented Aug 18, 2024

@Thirumalai-Shaktivel can you please tell what an appropriate title for this PR would be?

@tanay-man
Copy link
Copy Markdown
Contributor Author

tanay-man commented Aug 18, 2024 •
edited
Loading

fixes #2799, #2797

@Thirumalai-Shaktivel Thirumalai-Shaktivel changed the title Inheritance Initial implementation of Inheritance and Polymorphic functions Aug 18, 2024
@Thirumalai-Shaktivel
Copy link
Copy Markdown
Collaborator

Thirumalai-Shaktivel commented Aug 18, 2024

Thanks for the changes. I have set this PR to auto-merge

@Thirumalai-Shaktivel Thirumalai-Shaktivel merged commit 5a66456 into lcompilers:main Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Thirumalai-Shaktivel Thirumalai-Shaktivel Thirumalai-Shaktivel approved these changes

@certik certik certik approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tanay-man @certik @Thirumalai-Shaktivel