Skip to content

fix #13954: Unnamed bit fields are removed. #7611

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

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

Conversation

ludviggunne
Copy link
Collaborator

@ludviggunne ludviggunne commented Jun 19, 2025

No description provided.

@ludviggunne ludviggunne force-pushed the 13954 branch 8 times, most recently from 263ea07 to 3f9f968 Compare June 23, 2025 12:00
@ludviggunne ludviggunne marked this pull request as ready for review June 23, 2025 14:53
@ludviggunne ludviggunne force-pushed the 13954 branch 2 times, most recently from 5859d52 to 491e305 Compare June 24, 2025 13:16
@danmar
Copy link
Owner

danmar commented Jun 24, 2025

what does the test-my-pr.py say? are there diffs..?

@ludviggunne
Copy link
Collaborator Author

what does the test-my-pr.py say? are there diffs..?

I've only run it with e2764f0 so far, so I should probably run it again. There are some diffs, most of them seem to be eliminated false positive similar to #13653 & #13850.

tok->deleteNext(4 + tokDistance(tok, typeTok) - 1);
goback = true;
const std::size_t id = anonymousBitfieldCounter++;
const std::string name = "anonymous@" + std::to_string(id);
Copy link
Owner

Choose a reason for hiding this comment

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

I can accept that we use @ here. but the unfortunate side is that we also use @ in debug output to show varid.
But I think we developers can adapt.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I figured a colon might be confusing since we're dealing with bitfields. Is there some other character that doesn't appear in other contexts?

testBitfields("unsigned int a : 16;\n"
"unsigned int : 0;\n"
"unsigned int b : 16;\n",
8);
Copy link
Owner

Choose a reason for hiding this comment

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

These test cases looks unfortunate to me. It's platform dependent what the sizeof is for this struct isn't it?
Does the standard say what happens when a bitfield has size 0?

Copy link
Collaborator Author

@ludviggunne ludviggunne Jun 25, 2025

Choose a reason for hiding this comment

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

Yep, from cppreference:

The special unnamed bit-field of size zero can be forced to break up padding. It specifies that the next bit-field begins at the beginning of its allocation unit

But yes, the size can vary since the size of int can vary.

Copy link
Owner

Choose a reason for hiding this comment

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

i.e. I would assume there are compilers that will determine that the size is 4 bytes here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Iv'e added code to force using unix 64 platform in the tests.

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.

2 participants