Skip to content

Comments

Implemented visit_Return for freeing variables#2412

Merged
certik merged 2 commits intolcompilers:mainfrom
anutosh491:visit_return
Nov 9, 2023
Merged

Implemented visit_Return for freeing variables#2412
certik merged 2 commits intolcompilers:mainfrom
anutosh491:visit_return

Conversation

@anutosh491
Copy link
Collaborator

@anutosh491 anutosh491 commented Nov 9, 2023 •
edited
Loading

Fixes #2411

We need to take care of two things

  1. If we have a function returning something, we would like to free all local basic variables before returning
  2. As we are retuning the _lpython_return_variable I don't think so we need to free that.

Now we would have something like

                                    (SubroutineCall
                                        2 basic_free_stack
                                        2 basic_free_stack
                                        [((Var 3 stack0))]
                                        ()
                                    )
                                    (SubroutineCall
                                        2 basic_free_stack
                                        2 basic_free_stack
                                        [((Var 3 stack1))]
                                        ()
                                    )
                                    (Return)]
                                    (Var 3 _lpython_return_variable)

Where stack0 and stack1 might be some local variables used inside the function, then followed by the Return statement.

Thirumalai-Shaktivel
Thirumalai-Shaktivel reviewed Nov 9, 2023
View reviewed changes
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.

I think it's fine, thank you!

@certik
Copy link
Contributor

certik commented Nov 9, 2023

Can you add a test?

Why not calling the function to subroutine pass?

@certik certik mentioned this pull request Nov 9, 2023
Closed
certik
certik approved these changes Nov 9, 2023
View reviewed changes
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 it's fine.

@certik
Copy link
Contributor

certik commented Nov 9, 2023

This is fixing memory leaks.

@certik certik merged commit f0b2a49 into lcompilers:main Nov 9, 2023
@anutosh491 anutosh491 deleted the visit_return branch November 10, 2023 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@certik certik certik approved these changes

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

Assignees

No one assigned

Labels

symbolic

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Call basic_free_stack before return statement

3 participants

@anutosh491 @certik @Thirumalai-Shaktivel