-
Notifications
You must be signed in to change notification settings - Fork 618
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
Support for labels with no statement #562
base: main
Are you sure you want to change the base?
Conversation
…qual. Add test corroborating equality of ASTs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be introducing a large number of new shift/reduce conflicts for the parser, and it's clear why!
pycparser/c_parser.py
Outdated
@@ -1580,6 +1580,14 @@ def p_labeled_statement_3(self, p): | |||
""" labeled_statement : DEFAULT COLON pragmacomp_or_statement """ | |||
p[0] = c_ast.Default([p[3]], self._token_coord(p, 1)) | |||
|
|||
def p_labeled_statement_4(self, p): | |||
""" labeled_statement : ID COLON SEMI""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this rule (_4) should be necessary, since a statement can already be empty. Why do you need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the test, there's no reason to create a separate class just for this test. You can add a test_
function to the existing class.
Also, you're only checking that the code compiles properly without verifying anything about the AST. pycparser could completely ignore the label here and your test would still pass.
You probably want something similar to test_pragmacomp_or_statement but much simpler, that checks the right AST nodes with labels are parsed in a short function body.
This pull request is aimed at fixing the issue described here: #528
The testing I did seems fine, and I implemented it through two rules to ensure that the representation of
label:
andlabel:;
is the same.