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

C code generator failures with multiple qualifiers #433

Open
vit9696 opened this issue Sep 17, 2021 · 1 comment
Open

C code generator failures with multiple qualifiers #433

vit9696 opened this issue Sep 17, 2021 · 1 comment

Comments

@vit9696
Copy link
Contributor

vit9696 commented Sep 17, 2021

Sometimes C code generator fails "forgets" about qualifiers as shown in #431. One such example is mixing _Atomic with const and auto. Consider the following code:

self._assert_ctoc_correct('auto const _Atomic(int *) a;')

This code would fail as it would generate auto int * _Atomic a; losing const. In particular, the issue is that CGenerator does not print quals for Decl. I.e. the following AST is generated:

auto const _Atomic(int *) a;
FileAST(ext=[Decl(name='a',
                  quals=['const'
                        ],
                  storage=['auto'
                          ],
                  funcspec=[
                           ],
                  type=PtrDecl(quals=['_Atomic'
                                     ],
                               type=TypeDecl(declname='a',
                                             quals=[
                                                   ],
                                             type=IdentifierType(names=['int'
                                                                       ]
                                                                 )
                                             )
                               ),
                  init=None,
                  bitsize=None
                  )
            ]
        )

and then the generator turns it into:

auto int * _Atomic a;

FileAST(ext=[Decl(name='a',
                  quals=[
                        ],
                  storage=['auto'
                          ],
                  funcspec=[
                           ],
                  type=PtrDecl(quals=['_Atomic'
                                     ],
                               type=TypeDecl(declname='a',
                                             quals=[
                                                   ],
                                             type=IdentifierType(names=['int'
                                                                       ]
                                                                 )
                                             )
                               ),
                  init=None,
                  bitsize=None
                  )
            ]
        )

An obvious fix is to add quals printing in def _generate_decl(self, n):.

diff --git a/pycparser/c_generator.py b/pycparser/c_generator.py
index ded8c65..5a977e5 100644
--- a/pycparser/c_generator.py
+++ b/pycparser/c_generator.py
@@ -418,6 +418,7 @@ class CGenerator(object):
         s = ''
         if n.funcspec: s = ' '.join(n.funcspec) + ' '
         if n.storage: s += ' '.join(n.storage) + ' '
+        if n.quals: s += ' '.join(n.quals) + ' '
         s += self._generate_type(n.type)
         return s

But that route is a bit complicated as it leads to duplicating quals, e.g. _Atomic int x; gives _Atomic _Atomic int x; now. This feels wrong in the source, because I see the _Atomic qualifier is already present twice in the AST in the first place:

FileAST(ext=[Decl(name='x',
                  quals=['_Atomic'
                        ],
                  storage=[
                          ],
                  funcspec=[
                           ],
                  type=TypeDecl(declname='x',
                                quals=['_Atomic'
                                      ],
                                type=IdentifierType(names=['int'
                                                          ]
                                                    )
                                ),
                  init=None,
                  bitsize=None
                  )
            ]
        )

However, it is not clear what was the original intention:

  1. To duplicate quals in both Decl and TypeDecl but use Decl for user interaction only for extra verbosity?
  2. To have different notions, with duplicated quals in some places being accidental?

I tried prototyping writing a fix assuming it is (1), but it was not immediately successful as rather many tests started to fail, and the comparison did not work in the first place:

diff --git a/pycparser/ast_transforms.py b/pycparser/ast_transforms.py
index 367dcf5..f4f4786 100644
--- a/pycparser/ast_transforms.py
+++ b/pycparser/ast_transforms.py
@@ -134,9 +134,16 @@ def fix_atomic_specifiers(decl):
     if typ.declname is None:
         typ.declname = decl.name
 
+    return _fix_const_qualifiers(decl)
+
+def _fix_const_qualifiers(decl):
+    if isinstance(decl, c_ast.Decl) and decl.quals:
+        for qual in decl.quals:
+            if qual not in decl.type.quals:
+                decl.type.quals.append(qual)
+        # decl.quals = []
     return decl
@eliben
Copy link
Owner

eliben commented Sep 20, 2021

Indeed, this requires deeper investigation/thought. I'm not sure the quals on Decl are necessary, but maybe I'm missing something. In a perfect world there would be no reason for the duplication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants