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

Fixed #11807 - dump: C typedef enum not in <typedef-info> #5783

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

Conversation

nicoHarel
Copy link

  • Enum typedefs were removed from simplifyTypedefCpp "tok" list instead of unions.

As mentioned by Daniel here, the latest cppcheck version seems to not be affected by the issue anymore. But this is the fix nonetheless.

It was tested on the newest version (no effect) and an older version (commit 579938a on main).

- Enum typedefs were removed from simplifyTypedefCpp "tok" list instead of unions
@nicoHarel
Copy link
Author

Ok so apparently this makes an enum test fail. I don't really understand what's the test purpose nor what it's testing.

@chrchr-github
Copy link
Collaborator

With your fix applied, we produce invalid code like namespace N { enum E { } ; } void g ( int ) ; void f ( ) { g ( sizeof ( enum enum E ) ) ; } for certain inputs.
You said that the original problem is already fixed anyway? It would be nice to add a test for that, so regressions can be avoided in the future.

@nicoHarel
Copy link
Author

With your fix applied, we produce invalid code like namespace N { enum E { } ; } void g ( int ) ; void f ( ) { g ( sizeof ( enum enum E ) ) ; } for certain inputs. You said that the original problem is already fixed anyway? It would be nice to add a test for that, so regressions can be avoided in the future.

Yes, that makes sense actually. Since the new version adds the enum typedef in some other way that I'm not familiar with, it is very possible enum typedefs are now added twice with this fix.

I'll look into how the tests work.

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.

3 participants