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

Generate flexible array members the c99 way instead of the GNU way #994

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

Conversation

serge-sans-paille
Copy link

There a re three ways to express flexible array members:

  1. char fam[0];

  2. char fam[1];

  3. char fam[];

  4. is the only standard way (in c99, but supported in c++ as an extension), the other two are GNU syntax (still supported in clang though).

Let's generate the standard syntax by default, while leaving it possible to use 1. through struct.gnu_flexible_array_members

Cython only supports the 1. mode.

@serge-sans-paille serge-sans-paille force-pushed the feature/flexible-array-members branch from 2f376eb to 05cde18 Compare August 15, 2024 13:27
#[repr(C)]
struct WithFlexibleArrayMember {
x: i32,
y: [i8; 0],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you test with some other field afterwards too? Not sure what the behavior there should be, maybe it's fine.

Copy link
Author

Choose a reason for hiding this comment

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

I had to rework the whole approach. I've added a specific type to represent flexible array members, and that makes everything much more consistent.
I'm unsure about the idiomaticity of my rust code, don't hesitate to bash me :-)

@serge-sans-paille serge-sans-paille force-pushed the feature/flexible-array-members branch from 05cde18 to 2b2e64b Compare August 16, 2024 11:10
@serge-sans-paille
Copy link
Author

@emilio even if that change is correct from a compiler point of view, it would actually break mozilla code if gnu behavior is of by default because the firefox codebase is making an optimistic use of flexible array members. I guess it's still good to merge this, but with gnu behavior on by default. What do you think?

@serge-sans-paille serge-sans-paille force-pushed the feature/flexible-array-members branch from 2b2e64b to 4697c73 Compare August 17, 2024 08:32
There a re three ways to express flexible array members:

1. char fam[0];
2. char fam[1];
3. char fam[];

3. is the only standard way (in c99, but supported in c++ as an
extension), the other two are GNU syntax (still supported in clang
though).

To avoid regression, let's generate the GNU syntax by default, while leaving it possible to
use 3. through struct.c99_flexible_array_members

Cython only supports the 1. mode.
@serge-sans-paille serge-sans-paille force-pushed the feature/flexible-array-members branch from 4697c73 to ff2e4ef Compare August 17, 2024 08:39
Copy link
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Yeah, seems fine to do this. Want to apply that nit? Otherwise I can.

write!(out, "[{}]", constant);

last_was_pointer = false;
}
CDeclarator::FlexibleArray() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's no need for (), just remove it from the enum declaration.

@emilio
Copy link
Collaborator

emilio commented Oct 27, 2024

@emilio even if that change is correct from a compiler point of view, it would actually break mozilla code if gnu behavior is of by default because the firefox codebase is making an optimistic use of flexible array members.

Can you elaborate? Do you have an example?

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