You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add support for subtractions contenting references as right hand side operands
Description
The VM reads let expressions like c - tmp as an addition instead of a subtraction, where c is a constant and tmp is a local variable. This is because currently only additions are supported. This is not an issue with variables declared with the let keyword because those are resolved at compile time.
This PR introduces a little refactor to allow subtractions with references as the rhs operand. It does it by saving the sign with which the reference is defined.
I think this is OK as it's a simple and fast fix, but we should create an issue to tackle the underlying problem later on: supporting arbitrary expressions like cairo-lang does. I think we would eventually need to define an AST for the expression.
Since the compiler simplifies the expression whenever it can, there would be no problem in let expressions which only use constants or even variables defined with the let keyword. However, when we use variables defined with the local keyword this is not the case anymore because in doing so we are introducing the use of references.
PR looks good but the description could be rephrased, I think it's not clear what's the issue. It can be simplified to something like:
The VM reads let expressions as c - tmp as an addition instead of a subtraction, where c is a constant and tmp is a local variable.
Then it can describe why this happens.
Also:
Thus, only supporting additions would make expressiones like this one: 17 - [fp] to equivalent to 17 + [fp] because the sign of the reference is never taken into account.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add support for subtractions contenting references as right hand side operands
Description
The
VMreadsletexpressions likec - tmpas an addition instead of a subtraction, wherecis a constant andtmpis alocalvariable. This is because currently only additions are supported. This is not an issue with variables declared with theletkeyword because those are resolved at compile time.This PR introduces a little refactor to allow subtractions with references as the rhs operand. It does it by saving the sign with which the reference is defined.
Checklist