search

hpyproject / hpy

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

Use standard type ssize_t to define HPy_ssize_t. #426

Open fangerer opened 1 year ago

fangerer commented 1 year ago

This is the follow-up issue of #343 .

We aim to use fixed-width integers in the ABI to improve compatibility and portability. However, HPy assumes in several and crucial places that sizeof(HPy_ssize_t) == sizeof(Py_ssize_t).

In February 2023's dev call, we decided to not use a fixed-width integer like int64_t for HPy_ssize_t now since it is a bit more involved.

Instead, we will use do typedef ssize_t HPy_ssize_t for now.

steve-s commented 1 year ago

My main motivation for the original issue was that the ABI would be precisely specified including the bit width of all the arguments/return values/struct fields. Unfortunately, I don't remember exactly the discussion from the linked dev call, but just for the record: I think that switching HPy_ssize_t to ssize_t does not bring anything w.r.t. ABI stability. Maybe it has some other advantages? I want to make sure we do not purse something that does not solve the underlying motivation for using fixed size integers.

fangerer commented 1 year ago

Sorry for the late answer. I saw your comment but forgot ...

I think that switching HPy_ssize_t to ssize_t does not bring anything w.r.t. ABI stability

That's correct. IIRC, Antonio suggested this just to get away from Py_ssize_t (kind of as a first step). And Py_ssize_t just exists because older compilers just don't have ssize_t (only size_t).

I still plan to do something like:

typedef int64_t HPy_ssize_t

The main problem really is the assumption sizeof(HPy_ssize_t) == sizeof(Py_ssize_t). IMO, it is possible to use int64_t but if the assumption is then violated, we need to do proper conversions. I would assume that in most cases, the assumption still holds and there won't be any additional overhead but that just not guaranteed.

When converting, we also have the problem that we need to raise an error if the conversion fails (e.g. in case of an overflow). This needs to happen in the generated CPython trampolines which is a performance-critical place.

Btw. I already started to do all that. I will push the branch.

steve-s commented 1 year ago

When converting, we also have the problem that we need to raise an error if the conversion fails (e.g. in case of an overflow).

Right, thinking about it: however maybe not all such functions can even raise now, so that would be API change.