Skip to content

Yul expression simplifier: Don't substitute out of scope variables #16161

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 1 commit into from
Aug 18, 2025

Conversation

clonker
Copy link
Member

@clonker clonker commented Aug 12, 2025

In some circumstances (in particular if not the AST is not in SSA form), it could happen that the ExpressionSimplifier would substitute out-of-scope variables. From what I could tell, it seems impossible that this occurs in the default sequence, though.

Fixes #16155.

@clonker clonker force-pushed the fix_invalid_yul_from_expression_simplifier branch 2 times, most recently from 2269eb4 to 9ad50e3 Compare August 14, 2025 14:53
matheusaaguiar
matheusaaguiar previously approved these changes Aug 14, 2025
Copy link
Collaborator

@matheusaaguiar matheusaaguiar left a comment

Choose a reason for hiding this comment

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

LGTM.
I guess it would be best to remove the command line test as you mentioned in the matrix channel.

@clonker clonker force-pushed the fix_invalid_yul_from_expression_simplifier branch from 9ad50e3 to b22b2a6 Compare August 15, 2025 07:07
In some circumstances (in particular if not the AST is not in SSA form), it could happen that the ExpressionSimplifier would substitute out-of-scope variables.
@clonker clonker force-pushed the fix_invalid_yul_from_expression_simplifier branch from b22b2a6 to 116735f Compare August 15, 2025 08:32
Copy link
Collaborator

@nikola-matic nikola-matic left a comment

Choose a reason for hiding this comment

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

It's good to merge, although I would still prefer for the (optional) external tests to be passing, although not sure how likely we'll be to fix that in the short term.

@clonker clonker merged commit 0e8d07d into develop Aug 18, 2025
72 of 74 checks passed
@clonker clonker deleted the fix_invalid_yul_from_expression_simplifier branch August 18, 2025 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Yul] Expression simplifier can produce invalid Yul code
4 participants