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

Parsing implemented with all tests passing! 🎉 #25

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Parsing implemented with all tests passing! 🎉 #25

wants to merge 24 commits into from

Conversation

@hrszpuk hrszpuk added the enhancement New feature or request label Jan 6, 2025
@hrszpuk hrszpuk self-assigned this Jan 6, 2025
Copy link
Member Author

@hrszpuk hrszpuk left a 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 😎.

CMakeLists.txt Show resolved Hide resolved
return "ExpressionNode(op: " + op.to_string() + ")";
}

virtual std::variant<int, std::string, bool> eval() const = 0;
Copy link
Member Author

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?

Copy link
Member Author

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!

Copy link
Member Author

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!

Copy link
Member Author

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.

Copy link
Member Author

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;
Copy link
Member Author

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") + ")";
Copy link
Member Author

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.

Copy link
Member Author

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")
Copy link
Member Author

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.

Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Ready
Development

Successfully merging this pull request may close these issues.

1 participant