Restructure Definitions and Assigns in preparation for writing to arr… #28526
Conversation
This file contains 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
mikebenfield
force-pushed
the
restructure-defs-public
branch
2 times, most recently
from
0295f6e to
69b0e45
Compare
d0cd
reviewed
Mar 14, 2025
|
Oh also the commit message is backward - I didn't remove |
d0cd
reviewed
Mar 25, 2025
| } | ||
|
|
||
| fn visit_assign(&mut self, _input: &'a AssignStatement) -> String { | ||
| panic!("DefinitionStatement's should not exist in SSA form.") |
There was a problem hiding this comment.
Suggested change
| panic!("DefinitionStatement's should not exist in SSA form.") | |
| panic!("AssignStatement's should not exist in SSA form.") |
d0cd
reviewed
Mar 25, 2025
| @@ -35,8 +35,8 @@ impl Assigner { | |||
|
|
|||
| /// Constructs the assignment statement `place = expr;`. | |||
| /// This function should be the only place where `AssignStatement`s are constructed. | |||
There was a problem hiding this comment.
Suggested change
| /// This function should be the only place where `AssignStatement`s are constructed. | |
| /// This function should be the only place where `DefinitionStatement`s are constructed. |
d0cd
reviewed
Mar 25, 2025
| @@ -57,12 +57,13 @@ impl AssignerInner { | |||
|
|
|||
| /// Constructs the assignment statement `place = expr;`. | |||
| /// This function should be the only place where `AssignStatement`s are constructed. | |||
There was a problem hiding this comment.
Suggested change
| /// This function should be the only place where `AssignStatement`s are constructed. | |
| /// This function should be the only place where `DefinitionStatement`s are constructed. |
d0cd
reviewed
Mar 25, 2025
| fn reconstruct_definition(&mut self, _: DefinitionStatement) -> (Statement, Self::AdditionalOutput) { | ||
| unreachable!("`DefinitionStatement`s should not exist in the AST at this phase of compilation.") | ||
| fn reconstruct_definition(&mut self, mut input: DefinitionStatement) -> (Statement, Self::AdditionalOutput) { | ||
| // Check the lhs of the assignment to see any of variables are used. |
There was a problem hiding this comment.
Suggested change
| // Check the lhs of the assignment to see any of variables are used. | |
| // Check the lhs of the definition to see any of variables are used. |
d0cd
reviewed
Mar 25, 2025
| @@ -1,14 +1,8 @@ | |||
| namespace = "Compile" | |||
| expectation = "Fail" | |||
| outputs = [""" | |||
| Error [ETYC0372000]: invalid assignment target | |||
| --> compiler-test:5:9 | |||
| Error [EPAR0370005]: expected ; -- found '=' | |||
There was a problem hiding this comment.
Is there a way this error can be made more informative?
d0cd
approved these changes
Mar 25, 2025
|
Which documentation? You're referring to comments in the code? |
mikebenfield
force-pushed
the
restructure-defs-public
branch
from
80a6ff2 to
1cb7dc3
Compare
…ays, etc. Specifically: - AssociatedConstant and AssociatedFunction are moved out of AccessExpression to become variants of Expression. - A new type DefinitionPlace is the place in a DefinitionStatement. - ConstantDeclaration is gone - Definition can handle its case. - SSA pass now produces Definitions instead of Assigns. - The Place in an AssignStatement is now an Identifier. (In the future it will become a new type, capable of handling writes to arrays, structs, and tuples.)
mikebenfield
force-pushed
the
restructure-defs-public
branch
from
1cb7dc3 to
4834474
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Restructure Definitions and Assigns in preparation for writing to arrays, etc.
Specifically:
AssociatedConstant and AssociatedFunction are moved out of AccessExpression to become variants of Expression.
A new type DefinitionPlace is the place in a DefinitionStatement.
ConstantDeclaration is gone - Definition can handle its case.
SSA pass now produces Definitions instead of Assigns.
The Place in an AssignStatement is now an Identifier. (In the future it will become a new type, capable of handling writes to arrays, structs, and tuples.)