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

Initial implementation of Inheritance and Polymorphic functions #2801

Merged
merged 2 commits into from
Aug 18, 2024

Conversation

tanay-man
Copy link
Contributor

No description provided.

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.

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

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

Thirumalai-Shaktivel commented Aug 13, 2024

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
Contributor Author

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

@Thirumalai-Shaktivel
Copy link
Collaborator

Can you please rebase the branch with main?

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

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

@Thirumalai-Shaktivel
Copy link
Collaborator

Also, change the PR heading to appropriate description

@Thirumalai-Shaktivel
Copy link
Collaborator

Also, let's clean up the commit messages.

@Thirumalai-Shaktivel
Copy link
Collaborator

Thirumalai-Shaktivel commented Aug 18, 2024

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

@tanay-man
Copy link
Contributor Author

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

@tanay-man
Copy link
Contributor Author

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

@tanay-man
Copy link
Contributor Author

tanay-man commented Aug 18, 2024

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
Collaborator

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
17 of 18 checks passed
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.

3 participants