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

Introduce operator enum in Parser #4

Closed
54k1 opened this issue Apr 11, 2021 · 3 comments
Closed

Introduce operator enum in Parser #4

54k1 opened this issue Apr 11, 2021 · 3 comments

Comments

@54k1
Copy link
Contributor

54k1 commented Apr 11, 2021

Currently, for binary and unary expressions, the operator is being set as a Token. However, this can be simplified by introducing new enum types: BinaryOperator, UnaryOperator, AssignmentOperator, SetIndexOperator.

Example:

enum BinaryOperator {
    Add, Sub, Div, Mul, Mod, Equality, // ... etc
}
enum UnaryOperator {
    Add, Sub
}

// Binary Expression would be
struct Binary {
    pub left: Box<Expr>,
    pub right Box<Expr>,
    pub operator: BinaryOperator
}
// And similarly Unary Expression, and so on..
  1. These would be less bloated than having a token in each expression.
  2. Less Error prone since you get compile time check when setting the operator in the expressions. For instance, you can't set the BinaryOperator to some arbitrary token. I'm sure the appropriate checks are being made, however, this is cleaner IMO.

What do you think @tuqqu?

@54k1
Copy link
Contributor Author

54k1 commented Apr 11, 2021

@tuqqu, I have created a draft PR: #5 to show the proof of concept. Let me know if any improvements can be made so that I can start updating other expressions to use operators.

@54k1
Copy link
Contributor Author

54k1 commented Apr 11, 2021

To provide meaningful error messages, we must have access to the token location. Replacing Token with these operator enums does not make sense in this case.

@54k1 54k1 closed this as completed Apr 11, 2021
@tuqqu
Copy link
Owner

tuqqu commented Apr 13, 2021

Yes, basically we always need a Token to rely on. I see the profit of having proper enums for each type of an operator, but something is to be done with token metadata (location and later even a file path it was in, when we have file imports).
For simplicity I'd leave it as is.

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

No branches or pull requests

2 participants