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

Allow %prec to define a token in the grammar. #453

Merged
merged 1 commit into from
May 15, 2024

Conversation

ltratt
Copy link
Member

@ltratt ltratt commented May 15, 2024

Previously if %prec 'x' was the first mention of the token x then cfgrammar would say "'x' isn't a token" which is clearly incorrect. This commit is maybe best thought of as a quick fix of sorts: it makes %prec 'x' define a token x if it doesn't already exist.

However -- and my memory is completely fuzzy as to whether this is a Yacc compatibility thing or not -- this means that %prec 'x' and %prec x' (i.e. with and without quote marks) have the semantic meaning because parse_token` treats both interchangeably. This seems consistent with other parts of the grammar even if a little odd.

I believe this fixes #450.

Previously if `%prec 'x'` was the first mention of the token `x` then
cfgrammar would say "'x' isn't a token" which is clearly incorrect.
This commit is maybe best thought of as a quick fix of sorts: it makes
`%prec 'x'` define a token `x` if it doesn't already exist.

However -- and my memory is completely fuzzy as to whether this is a
Yacc compatibility thing or not -- this means that `%prec 'x'` and
`%prec x' (i.e. with and without quote marks) have the semantic meaning
because `parse_token` treats both interchangeably. This seems consistent
with other parts of the grammar even if a little odd.
@ratmice
Copy link
Collaborator

ratmice commented May 15, 2024

I should be honest and say I'm not really familiar with %prec or any of the associativity declarations, as I tend towards manually resolving ambiguity even if doing so it isn't terribly convenient.

This seems okay for a quick fix though, in that it seems low impact and isolated to %prec, which specifically does not affect parse_token which I would be more concerned about.

One other part of grmtools (albeit there isn't an equivalent in yacc) where quotes have semantic meaning and aren't interchangeable is %expect-unused, which first tries to parse it as a name then failing that tries to parse it as a token, which in that path means generally unquoted tokens will be treated as a name.

Anyhow i'm happy to approve if you think it is ready.

@ltratt
Copy link
Member Author

ltratt commented May 15, 2024

One other part of grmtools (albeit there isn't an equivalent in yacc) where quotes have semantic meaning and aren't interchangeable is %expect-unused, which first tries to parse it as a name then failing that tries to parse it as a token, which in that path means generally unquoted tokens will be treated as a name.

Good point. I wonder if there should be another PR which a) checks exactly what yacc does in these cases (I vaguely remember it allows either case, but I might well be wrong) b) checks the code for precise compatibility with yacc? I'm not sure I'll get to that very soon, so at least this PR nudges us in a better-than-the-status-quo direction...

@ratmice
Copy link
Collaborator

ratmice commented May 15, 2024

There is a bit more about prec in https://pubs.opengroup.org/onlinepubs/7908799/xcu/yacc.html
But here is the relevant bits that would affect quoting

The reserved symbol %prec can appear immediately after the body of the grammar rule and can be followed by a token name or a literal. It will cause the precedence of the grammar rule to become that of the following token name or literal.

Edit:
So it definitely seems like it allows either case, but it seems pretty silent on the handling of this specific token/rule not found case.

@ratmice ratmice added this pull request to the merge queue May 15, 2024
Merged via the queue into softdevteam:master with commit 1cbbd2a May 15, 2024
2 checks passed
@ltratt ltratt deleted the prec_defines_a_token branch June 14, 2024 09:40
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.

lrpar:%prec not followed by token name
2 participants