Skip to content

Move actiont checking from codegen to ast validation. - #659

Open
ratmice wants to merge 6 commits into
softdevteam:masterfrom
ratmice:validate_yacckind
Open

Move actiont checking from codegen to ast validation.#659
ratmice wants to merge 6 commits into
softdevteam:masterfrom
ratmice:validate_yacckind

Conversation

@ratmice

@ratmice ratmice commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Here is an attempt to validate actiont earlier,
adding it to YaccGrammarError.

I'm uncertain it's possible to trigger for YaccKind::Grmtools I would imagine you'd get a syntax error before it could complete_and_validate. So I checked that with a manually constructed AST.

Comment thread lrpar/src/lib/ctbuilder.rs Outdated

@ratmice ratmice Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There is a similar error here that should probably also be moved in this patch

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Should be fixed in 9e956c2

Comment thread lrpar/src/lib/ctbuilder.rs Outdated

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ditto for this error.

@ratmice ratmice Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Should be fixed in 6313b96 and tested in ca3a9b1

And with that the diag variable becomes unused and can be removed from the last stages of codegen.

Comment thread cfgrammar/src/lib/yacc/parser.rs Outdated
InvalidString,
NoStartRule,
UnknownSymbol,
UnrecognizedDollarVariable,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Would UnrecognizedActionVariable be better?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I renamed it in c2d027c

) -> Result<(), YaccGrammarError> {
let kind_requires_actiont = matches!(
yacc_kind,
Some(YaccKind::Original(YaccOriginalActionKind::UserAction)) | Some(YaccKind::Grmtools)

@ratmice ratmice Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We could consider making this boolean check a method on YaccKind?
With a different name like YaccKind::requires_actions?

I think it might also simplify a similar check in CTParserBulder.

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