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

Add support for __attribute__(()) in enum/struct/union #76

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

douglas-raillard-arm
Copy link
Contributor

@douglas-raillard-arm douglas-raillard-arm commented Sep 1, 2023

Add support for GNU __attribute__(()) in enum/struct/union definitions, both on the type itself and on enumerators.

Fixes #42

@inducer
Copy link
Owner

inducer commented Sep 1, 2023

Thanks for the contribution! Could you fix the linter failure? Otherwise this LGTM.

@douglas-raillard-arm
Copy link
Contributor Author

@inducer I just realized that at least GCC accepts multiple __attribute__(()) on the same declaration. That PR works when they are next to each-other but obviously not if they are in multiple positions. I tried a production like that:

struct_or_union_specifier   : struct_or_union attributes_opt ID brace_open brace_close attributes_opt

But this only matches if there is an __attribute__(()) in each position, so that would lead to pretty bad combinatorial explosion of the number rules. Do you have any suggestion to fix that ?

@inducer
Copy link
Owner

inducer commented Sep 4, 2023

But this only matches if there is an __attribute__(()) in each position

Not sure I understand. Wouldn't each attributes_opt be optional individually?

@douglas-raillard-arm
Copy link
Contributor Author

Not sure I understand. Wouldn't each attributes_opt be optional individually?

Yes, I did not realize it actually worked because of another issue. I'm reworking the rules to allow __attribute__(()) in multiple position at once, I'll update the PR when it's ready.

Add support for GNU __attribute__(()) in enum/struct/union definitions,
both on the type itself and on enumerators.

Fixes inducer#42

Signed-off-by: Douglas Raillard <douglas.raillard@arm.com>
@douglas-raillard-arm
Copy link
Contributor Author

@inducer Unfortunately I stopped using pycparserext (instead of parsing a pre-existing header made by another tool, I made a tool that creates the header in the first place) and I don't have the bandwidth to fix pycparserext. Feel free to re-own this PR if someone else wants to take over.

I did not go super far in the rework of that PR, and I ended up with some sort of conflict where the attribute was parsed then swallowed up by pycparser as that bit of the declaration is deemed irrelevant on that path, and is therefore dropped on the floor. I'm going to push the WIP commit I was working on, but that commit completely breaks the PR (it can just be reverted to get the simpler but incomplete fix that I originally submitted)

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.

struct in a struct with __attribute__((packed))
2 participants