search

hpyproject / hpy

HPy: a better API for Python
https://hpyproject.org
MIT License
1.02k stars 52 forks source link

ambiguous struct fields in HPyType_Spec #454

Open mattip opened 11 months ago

mattip commented 11 months ago

Currently HPyType_Spec uses HPyType_BuiltinShape which is an enum and long in its typedef https://github.com/hpyproject/hpy/blob/f6114734b4a5d03cfb9d24d3e8c71f77e8803881/hpy/devel/include/hpy/hpytype.h#L102 https://github.com/hpyproject/hpy/blob/f6114734b4a5d03cfb9d24d3e8c71f77e8803881/hpy/devel/include/hpy/hpytype.h#L131

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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.