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

Change AST representation of emit statement #337

Closed
cburgdorf opened this issue Mar 25, 2021 · 1 comment
Closed

Change AST representation of emit statement #337

cburgdorf opened this issue Mar 25, 2021 · 1 comment
Assignees

Comments

@cburgdorf
Copy link
Collaborator

What is wrong?

The emit statement is currently represented in the AST as:

Emit {
        value: Node<Expr>,
    },

This suggests that the right hand side can be any expression such as emit get_some_event() or emit SomeEvent() if condition else OtherEvent().

However, that is not actually allowed and we reject these cases in the analyzer pass. If the right hand side is not actually allowed to be an expression (it is unclear to me if it ever could) then we should change the AST representation to something that prevents that in the first place.

How can it be fixed

  1. Probably change the Emit node to something with just name and args fields
  2. Change the parser to parse it according to the new representation
  3. Refactor the analyzer pass to get rid of the current checks that are concerned with syntax
@sbillig sbillig mentioned this issue Apr 21, 2021
3 tasks
@sbillig sbillig added the good first issue Good for newcomers label Apr 23, 2021
@cburgdorf cburgdorf self-assigned this Apr 26, 2021
@cburgdorf
Copy link
Collaborator Author

@sbillig just to let you know, I started working on this today to get familiar with the new parser. I should have something to review for you soon :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants