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

Some errors in "grammar.y" not caught by bison #1136

Closed
mingodad opened this issue Feb 15, 2024 · 9 comments
Closed

Some errors in "grammar.y" not caught by bison #1136

mingodad opened this issue Feb 15, 2024 · 9 comments

Comments

@mingodad
Copy link

While doing some tests with a script to convert grammar.y to a tree-sitter grammar I discovered what seems to be a copy and paste mistake that bison doesn't detect.

Here

: FOREACH_R optional_label '(' foreach_vars ':' expr ')' statement
we need to replace : by | (what seems to a copy and paste operation that forgot to replace : by |).

There was also some rules without the closing ; and mix of spaces and tabs, the lexer also was missing BANBANG definition and other minor problems (see my proposed fixes in the attached zip bellow).

gramlex.zip

@lerno
Copy link
Collaborator

lerno commented Feb 15, 2024

I'm unsure of some of those lexer changes, 1__000__000 is actually permitted currently. And I think there was something else there as well. I'll have a look. Keep in mind that the top level compile time if/switch is removed. Otherwise the grammar fixes were all good.

@mingodad
Copy link
Author

What do you mean by top level compile time if/switch is removed ?
My changes did it ?

@lerno
Copy link
Collaborator

lerno commented Feb 15, 2024

No I mean it's gone and I removed it from the dev branch already, but your grammar.y had it still.

@lerno
Copy link
Collaborator

lerno commented Feb 15, 2024

Why did you change to {HEX}+ wasn't that redundant?

@mingodad
Copy link
Author

Do you mean "Why did I changed from {HEX}+ to {HEX} ?
If so yes it was redundant.

@lerno
Copy link
Collaborator

lerno commented Feb 16, 2024

Sorry that was me reading things wrong. I think I updated with most of your changes now, but the 1__0 will not be invalid until 0.5.5

@mingodad
Copy link
Author

Thank you !
The _?{D|H|B} it's OK to stay _*{D|H|B}.

@lerno
Copy link
Collaborator

lerno commented Feb 16, 2024

I've updated in the dev branch already. It's fine if your parser only accepts _? I don't think anyone uses this feature anyway.

@lerno
Copy link
Collaborator

lerno commented Jun 18, 2024

I think I can close this.

@lerno lerno closed this as completed Jun 18, 2024
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

No branches or pull requests

2 participants