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

ambiguous struct fields in HPyType_Spec #454

Open
mattip opened this issue Oct 31, 2023 · 3 comments
Open

ambiguous struct fields in HPyType_Spec #454

mattip opened this issue Oct 31, 2023 · 3 comments
Assignees

Comments

@mattip
Copy link
Contributor

mattip commented Oct 31, 2023

Currently HPyType_Spec uses HPyType_BuiltinShape which is an enum and long in its typedef

unsigned long flags;

HPyType_BuiltinShape builtin_shape;

These are ambiguous since enum does not have to be int and long is 4 bytes on windows but 8 everywhere else.

Also the struct is not aligned, it needs padding around the HPyType_BuiltinShape to align on 8 bytes.

@fangerer
Copy link
Contributor

fangerer commented Dec 7, 2023

Sorry, overlooked this issue.
However, I don't really understand what you mean. Do you mean that, for ABI stability, we should use fixed-width types like uint64_t or similar?

@mattip
Copy link
Contributor Author

mattip commented Dec 7, 2023

We should not have enum nor 'long' in publicly-facing API, since thy are not well defined. Something like uint64_t would be preferable, and would align the struct.

@fangerer
Copy link
Contributor

fangerer commented Dec 7, 2023

Agreed. IIRC, we used unsigned long for flags since we first tried to make our struct compatible with CPython's PyType_Spec but we dismissed that anyway.
For the enum, we basically didn't think about that. An enum just seemed to be the right thing since there is a limited set of valid values.

@fangerer fangerer self-assigned this Dec 7, 2023
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

No branches or pull requests

2 participants