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

Open a block and function scope for python functions #1731

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

oxisto
Copy link
Member

@oxisto oxisto commented Sep 25, 2024

This PR makes sure we open a new (block) scope for the function body. This is not a 1:1 mapping to python scopes, since python only has a "function scope", but in the CPG the function scope only comprises the function arguments, and we need a block scope to hold all local variables within the function body.

Copy link

sonarcloud bot commented Sep 25, 2024

@oxisto oxisto marked this pull request as draft September 25, 2024 17:25
Copy link
Contributor

@maximiliankaul maximiliankaul left a comment

Choose a reason for hiding this comment

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

Assuming that we don't want to change CPG behavior -> I'm ok with this workaround.

@oxisto
Copy link
Member Author

oxisto commented Sep 26, 2024

Assuming that we don't want to change CPG behavior -> I'm ok with this workaround.

I am still not 100 % convinced myself it this is a good idea or not.

@oxisto oxisto marked this pull request as ready for review September 26, 2024 10:42
@oxisto
Copy link
Member Author

oxisto commented Sep 26, 2024

Assuming that we don't want to change CPG behavior -> I'm ok with this workaround.

I am still not 100 % convinced myself it this is a good idea or not.

after discussion with @maximiliankaul we agree that this is the "best" possible workaround for the moment

@oxisto oxisto merged commit 1b91777 into main Sep 26, 2024
3 checks passed
@oxisto oxisto deleted the python-function-scope branch September 26, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants