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

Generate pointer types correctly #315

Merged
merged 2 commits into from
Mar 27, 2019
Merged

Conversation

amirgon
Copy link
Contributor

@amirgon amirgon commented Mar 26, 2019

I just realized a few things:

  • Pointer types are not generated correctly.
    Example:
from pycparser import c_parser, c_ast, c_generator
parser = c_parser.CParser()
gen = c_generator.CGenerator()
ast = parser.parse('const int ** const  x;')

>>> gen.visit(ast.ext[0])
'const int ** const x'
>>> gen.visit(ast.ext[0].type)
'const int'
>>> gen.visit(ast.ext[0].type.type)
'const int'
>>> gen.visit(ast.ext[0].type.type.type)
'const int'
>>> 

In this PR I fixed it and now it makes more sense:

>>> gen.visit(ast.ext[0])
'const int ** const x'
>>> gen.visit(ast.ext[0].type)
'const int ** const'
>>> gen.visit(ast.ext[0].type.type)
'const int *'
>>> gen.visit(ast.ext[0].type.type.type)
'const int'
  • There is code duplication between visit_ArrayDecl/visit_TypeDecl and _generate_type. In order to use _generate_type, all that is needed is to prevent it from emitting the declname when we only want to generate these decls.
    In this PR I removed this code duplication.

Also removed code duplication from visit_ArrayDecl and visit_TypeDecl by calling _generate_type instead, without emitting the declname.

Added tests for ptr type generation
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 for the fix! Just one minor nit

self._assert_ctoc_correct(src)
ast = parse_to_ast(src)
generator = c_generator.CGenerator()
self.assertEqual(generator.visit(ast.ext[0].type),'const int ** const')
Copy link
Owner

Choose a reason for hiding this comment

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

Please have a space after the comma in argument lists. Wrap the second argument if it doesn't fit in 80 lines (see examples elsewhere)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally I think that the strict 80 character limitation is archaic and irrelevant with today's display sizes.
Anyway... Fixed on 878527b.

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.

None yet

2 participants