pycparser icon indicating copy to clipboard operation
pycparser copied to clipboard

C code generator failures with multiple qualifiers

Open vit9696 opened this issue 4 years ago • 1 comments

Sometimes C code generator fails "forgets" about qualifiers as shown in https://github.com/eliben/pycparser/pull/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

vit9696 avatar Sep 17 '21 21:09 vit9696

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.

eliben avatar Sep 20 '21 13:09 eliben