Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

patch: fix NoneType error for 'visit_Case' function in c_generator.py… #548

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kmg3821
Copy link

@kmg3821 kmg3821 commented Jul 22, 2024

… when a case is empty by adding none type checker

Copy link
Owner

@eliben eliben left a comment

Choose a reason for hiding this comment

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

Thanks!

Would it be possible to add a test that triggers this? (fails before the PR, succeeds after)

@kmg3821
Copy link
Author

kmg3821 commented Jul 23, 2024

Thanks!

Would it be possible to add a test that triggers this? (fails before the PR, succeeds after)

Scenario :
C code --> AST --> C code

Input C code :

void foo(int n) {
switch (n)
{
case 0:
case 1:
printf("Hello");
break;
case 2:
printf("World");
default:
break;
}
}

Error occurs when converting AST into C code :

image

This happens because of the "case 0" statement, which is empty :

image

After adding the None type checker :
The following C code is successfully generated.
image

@eliben
Copy link
Owner

eliben commented Jul 23, 2024

Yes, I understand

My question is - can you add a test to https://github.com/eliben/pycparser/blob/main/tests/test_c_generator.py as part of your PR that triggers this behavior and verifies that the change fixes it

@kmg3821
Copy link
Author

kmg3821 commented Jul 24, 2024

I'm sorry for the confusion, but the problem is not with the c_generator.py.

The issue lies with the "to_dict" function in c_json.py within the examples folder, which saves None instead of an empty list for the case node type.

It appears that tests are not performed for the examples, so I did not add the test you requested.

However, if needed, I can add it.

Copy link
Owner

@eliben eliben left a comment

Choose a reason for hiding this comment

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

The PR name should be adjusted also to reflect what it's fixing

@mypalmike can you PTAL?

@@ -116,6 +116,8 @@ def to_dict(node):
for child_attr in child_attrs_of(klass):
if child_attr not in result:
result[child_attr] = None
if result['_nodetype'] == "Case" :
Copy link
Owner

Choose a reason for hiding this comment

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

do an if...else here?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants