-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Parsing implemented with all tests passing! 🎉 #25
base: main
Are you sure you want to change the base?
Conversation
Created tests for the parser
…-the-feature-set Set up ast and nodes for the feature set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are mostly done, some expected result strings are still placeholders. Some of the AST nodes aren't fully implemented, and obviously the parser declaration + implementation isn't done yet.
I might extend this pull request by a day or two on the project board. Other than that I'm still on track 😎.
return "ExpressionNode(op: " + op.to_string() + ")"; | ||
} | ||
|
||
virtual std::variant<int, std::string, bool> eval() const = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At what stage is eval()
going to be used? Maybe semantic analysis?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO(hrs): for loop AST node isn't fully implemented!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO(hrs): function call AST node isn't fully implemented!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO(hrs): identifier AST node isn't fully implemented!
Side note, does the identifier node hold any distinct purpose or could it be merged with the main expression node?? Token, left, and right feels like it should be enough for identifiers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually- having them as a distinct type might be useful for debugging and semantic analysis... hmm... I think I'll keep it for now, but if it isn't useful for the future we'll just merge it.
|
||
class VariableDeclarationNode : public StatementNode { | ||
public: | ||
Token identifier; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This nodes has an identifier and type attributes but could use an identifier node instead.
std::string to_string() const override { | ||
return "VariableDeclarationNode(identifier: " + identifier.to_string() | ||
+ ", type: " + type.to_string() | ||
+ ", is_const: " + (is_const ? "true" : "false") + ")"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Value isn't included in this to_string. We should just call to_string on the ExpressionNode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO(hrs): while loop AST node isn't fully implemented!
@@ -1,4 +1,6 @@ | |||
add_executable(run_tests lexer_test.cpp parser_test.cpp semantics_test.cpp) | |||
file(GLOB_RECURSE PARSER_TESTS "parser/*.cpp") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it'll be easy to port the lexer tests over, at least for the cmake file anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty program test file :( - TODO(hrs): fill the test file with a meanful, full-program test.
This will tick off the second check point for milestone 1.
Related issues
Related pull requests