cty icon indicating copy to clipboard operation
cty copied to clipboard

ssize_t is not a standard C type?

Open gnzlbg opened this issue 6 years ago • 13 comments

cc @retep998

The VC CRT does not provide ssize_t and libc bases its ssize_t on SSIZE_T from Windows API. I'd really rather not have ssize_t declared a truly universal type.

gnzlbg avatar Feb 20 '19 08:02 gnzlbg

It is a type defined by POSIX. It is not in the C standard.

The standard equivalent is ptrdiff_t.

Amanieu avatar Feb 20 '19 13:02 Amanieu

IMO we should just remove ssize_t from cty.

Amanieu avatar Feb 20 '19 13:02 Amanieu

Note that cty is not a crate with "type aliases to standard C types"; it's a crate with "Type aliases to C types like c_int for use with bindgen" so I would not remove any type alias that bindgen uses as that's the main use case. Whether that's the case for ssize_t with today's bindgen I do not know -- I haven't used bindgen in a while.

japaric avatar Feb 20 '19 14:02 japaric

Does bindgen generate a libc::ssize_t or cty::ssize_t ? I thought it always translated ssize_t to isize. cc @emilio could you comment on that ?

IMO we should just remove ssize_t from cty.

So for supported targets, we do know whether the target has an ssize_t type, and if that's the case, what type does it map to (e.g. isize). So we can always expose it reliably.

One question is whether ssize_t should be available in targets that do not have it like some windows targets. If you are targeting a target without ssize_t, I expect that you won't need ssize_t while using bindgen because the headers for this target wouldn't use it either. Otherwise, something is really messed up, e.g., the headers one is feeding to bindgen correspond to a different target.

Another question is what shall we do if the user tries to use cty on a target that we don't know anything about? Should we always assume that such targets always have an ssize_t = isize ? Or shall we allow users to customize this behavior, e.g., via a cargo feature, etc. ?

gnzlbg avatar Feb 20 '19 14:02 gnzlbg

Bindgen always translates ssize_t and ptrdiff_t to isize, yeah. I'm not quite sure on the reason why, if any, if you want that changed and have a reasonable argument I'm happy to take patches for that.

emilio avatar Feb 21 '19 13:02 emilio

I think we should tell bindgen to use the "<ctypes_crate>" here (whether from libc or the user specified crate / path) and I can send a PR for that.

Whether we should provide ssize_t here, I think, with the inclusion in libc in mind, that we can do that on platforms that support it, but we should not provide it otherwise. If someone complains that it is not available, we can explore what the issue is, and figure out how to solve it (e.g. we can add an optional cargo feature that enables these unconditionally or something).

How does that sound ?

gnzlbg avatar Feb 21 '19 13:02 gnzlbg

Sounds fine, but note that bindgen supports both libc and std. Does std provide some ssize_t alternative?

emilio avatar Feb 21 '19 16:02 emilio

We can make bindgen use libc::ssize_t when the user wants to use the libc / cty types, and isize otherwise for the time being.

I don't know if libstd provides ssize_t, but if it does, it would probably be in the neighborhood of std::os::raw.

gnzlbg avatar Feb 21 '19 23:02 gnzlbg

We can make bindgen use libc::ssize_t when the user wants to use the libc / cty types, and isize otherwise for the time being.

What's the point of that, if cty::ssize_t is always isize? Is there a case where ssize_t wouldn't match isize?

emilio avatar Feb 22 '19 00:02 emilio

@emilio We know that all targets that Rust currently supports, if they do provide ssize_t (some windows targets do not), then it is equal to isize.

I do not know if this is the case for all future targets that Rust will support (maybe somebody else knows what POSIX says about ssize_t?).

If we ever support a target for which, isize is 128-bit wide, but ssize_t is only 64-bit wide, then we will need to do a backwards incompatible change to cty, or libc, or both, and that was very painful for the whole ecosystem last time.

This doesn't impact bindgen much, as in, if bindgen chooses to always use isize, then if a new target pops up for which this does not hold, then we upgrade bindgen and call it a day. I don't expect that to be painful. But if we have to release a backwards incompatible version of libc because of this, then that would be very unfortunate.

gnzlbg avatar Feb 22 '19 00:02 gnzlbg

@japaric it appears that, at least for the time being, bindgen doesn't require ssize_t at all. So what do you think about providing it in cty for the targets we know today, and not providing it for unknown targets?

gnzlbg avatar Feb 22 '19 00:02 gnzlbg

I know at least one platform where this fails: On MSP430X microcontrollers, some toolchains (notably recent versions of the gcc toolchain) use an uint20_t for size_t and an int20_t for ssize_t

From Stack Overflow. We support MSP430 at the moment but not MSP430X; IDK if the latter is supposed to be considered a different architecture when it comes to conditional compilation, but sounds like it would need type isize = i20 in any case since it has 20-bit instructions and pointers.

So what do you think about providing it in cty for the targets we know today, and not providing it for unknown targets?

Sounds good to me. How should we specify the "the targets we know today"? A mix of cfg(arch) and cfg(os)?

japaric avatar Feb 23 '19 15:02 japaric

How should we specify the "the targets we know today"? A mix of cfg(arch) and cfg(os)?

Pretty much, this is how libc does it now, so we can probably adapt that. Even though it might not be technically necessary, we probably want to be conservative in the supported targets and do a minor semver version bump. If that happens to break somebody's workflow, we can then manually add support for more targets in a backwards compatible way.

EDIT: FWIW we managed to do this for libc by just being super careful and bumping the patch version a couple of months ago, so the minor version bump isn't strictly necessary. But currently cty does not have the same constraints than libc has, so I think I would prefer to do that anyways.

gnzlbg avatar Feb 23 '19 15:02 gnzlbg