Skip to content

New validators - #257

Open
Nemoumbra wants to merge 3 commits into
WerWolv:masterfrom
Nemoumbra:new-validators
Open

Nemoumbra wants to merge 3 commits into
WerWolv:masterfrom
Nemoumbra:new-validators

Conversation

@Nemoumbra

Copy link
Copy Markdown
Contributor

Follow up to the 2nd PR: 2 validators for the pipeline. The old one has been removed.
Also enables the tests I added in the 1st PR... except for the duplicate enum test.
We already discussed this issue and how to resolve it, so it's not in the scope of this PR.
Let's see if it holds water.

#include <pl/core/ast/ast_node_match_statement.hpp>
#include <pl/core/ast/ast_node_conditional_statement.hpp>

namespace pl::core {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell, this only handles control flow statements in functions but not inside types, correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

}

void ScopeValidator::visit(ast::ASTNodeEnum &node) {
// TODO: enums are broken

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broken how? This definitely needs to be fixed before merge

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broken "duplicate enum entries should be handled in the parser as we discussed in Discord".

@WerWolv

WerWolv commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Code wise this looks great!
We'll need to fix the issues in the current patterns though before merging

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

2 participants