Skip to content

Comments

Parsing implemented with all tests passing! 🎉 #25

Open
hrszpuk wants to merge 24 commits intomainfrom
dev
Open

Parsing implemented with all tests passing! 🎉 #25
hrszpuk wants to merge 24 commits intomainfrom
dev

Conversation

@hrszpuk
Copy link
Member

@hrszpuk hrszpuk commented Jan 6, 2025

@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 😎.

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