Skip to content

Improve C++ codegen by using last precedence #100

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

Merged
merged 10 commits into from
Feb 23, 2022

Conversation

namannimmo10
Copy link
Collaborator

No description provided.

@namannimmo10 namannimmo10 marked this pull request as draft February 3, 2022 12:47
@namannimmo10 namannimmo10 marked this pull request as ready for review February 3, 2022 16:44
@namannimmo10 namannimmo10 requested a review from certik February 3, 2022 21:57
@certik
Copy link
Contributor

certik commented Feb 3, 2022

You have to update test results.

@certik
Copy link
Contributor

certik commented Feb 19, 2022

@namannimmo10, if you want, you can try to finish this one. It shouldn't be difficult.

@namannimmo10
Copy link
Collaborator Author

I'll try to complete this one soon.

@namannimmo10
Copy link
Collaborator Author

namannimmo10 commented Feb 21, 2022

Ready? Thanks for your comments!

@certik
Copy link
Contributor

certik commented Feb 22, 2022

I haven't gone over the other expr visitors, but we need to carefully go over every expr visitor and ensure we assign to last_expr_precedence, or add a comment.

@namannimmo10 namannimmo10 requested a review from certik February 22, 2022 19:23
@certik
Copy link
Contributor

certik commented Feb 22, 2022

Last minor update I found. After that, do you mind rebasing this PR to a more logical set of commits? It looks like we don't have any comments, we always assign the last precedence, so you can probably squash the last few commits together and rewrite the commit log.

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 looks great, thanks! After this is merged, I'll port it to LFortran where we have a lot more C++ tests and if there are any bugs, I'll report back.

@namannimmo10 namannimmo10 merged commit 4f0198d into lcompilers:main Feb 23, 2022
@namannimmo10 namannimmo10 deleted the last_prec branch February 23, 2022 00:04
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