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

Relax unshadowing for let defs #1444

Merged
merged 8 commits into from
May 22, 2024
Merged

Relax unshadowing for let defs #1444

merged 8 commits into from
May 22, 2024

Conversation

bugarela
Copy link
Collaborator

@bugarela bugarela commented May 21, 2024

Hello :octocat:

This makes unshadowing smarter so it doesn't have to rename all names, only the ones that are actually shadowing another name.

Unfortunately I could just do it for the let definitions, not for lambdas. See #1443. But this is the most important part, because it fixes names for nondets and nondetPicks looks good (see the test).

I found a couple of problems in our definition collection through testing this, since we rely on more collected metadata to decide what to unshadow now. So there are some small fixes for those as well.

  • Tests added for any new code
  • Documentation added for any new functionality
  • Entries added to the respective CHANGELOG.md for any new functionality
  • Feature table on README.md updated for any listed functionality

@bugarela bugarela marked this pull request as ready for review May 21, 2024 19:29
Copy link
Member

@p-offtermatt p-offtermatt left a comment

Choose a reason for hiding this comment

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

LGTM, great change.
It's really important to have the variable names for the nondets fixed and predictable to consume these in another program, so this is great to have.

@bugarela bugarela merged commit 709821b into main May 22, 2024
15 checks passed
@bugarela bugarela deleted the gabriela/soft-unshadowing branch May 22, 2024 10:55
This was referenced May 22, 2024
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.

2 participants