Move universal libc types to the root

[josephlr]

Per @alexcrichton's comment here, we should move the truely universal types back into the crate root. Let me know if you think some of these types are not universal (I've never seen a C-compiler use any definitions except the ones below).

This allows for crates like ring to use the libc crate while still supporting custom/unsupported targets (like x86_64-unknown-uefi).

Specifically the following code was moved from ::{unix, windows, cloudabi, redox, fuchsia, sgx, switch} to ::.

pub type int8_t = i8;	
pub type int16_t = i16;	
pub type int32_t = i32;	
pub type int64_t = i64;	
pub type uint8_t = u8;	
pub type uint16_t = u16;	
pub type uint32_t = u32;	
pub type uint64_t = u64;	

pub type c_schar = i8;	
pub type c_uchar = u8;	
pub type c_short = i16;	
pub type c_ushort = u16;	
pub type c_int = i32;	
pub type c_uint = u32;	
pub type c_float = f32;	
pub type c_double = f64;	
pub type c_longlong = i64;	
pub type c_ulonglong = u64;	
pub type intmax_t = i64;	
pub type uintmax_t = u64;	

pub type size_t = usize;	
pub type ptrdiff_t = isize;	
pub type intptr_t = isize;	
pub type uintptr_t = usize;	
pub type ssize_t = isize;

pub const INT_MIN: c_int = -2147483648;	
pub const INT_MAX: c_int = 2147483647;

This change also makes sure that all basic C types are referred to like ::c_int, this will make moving this code around in the future much easier.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[gnzlbg]

While this refactoring is correct for the targets that we currently have implemented, it isn't necessary correct for all targets that we might want to support. Also, types like int64_t are optional in C, that is, they are not guaranteed to be defined for a target.

So I don't know. I think I'd prefer for libc to contain "stuff" only for the targets that it actually supports, and to be empty otherwise. This allows crates that depend on it, but don't use it, to compile, and it errors on crates that try to use it in an unsupported platform, intptr_t and size_t don't necessarily need to have the same bit-width (e.g. on CHARI intptr_t is 128-bit wide but size_t is only 64-bit wide, etc.).

If we are going to add this, I'd prefer if this was "feature gated", e.g., behind a generic_ctypes cargo feature, so that users have to explicitly opt-in to this (generic ctypes that might not look like the ctypes in your platform, whose type might change once we add support for that platform).

This allows for crates like ring to use the libc crate while still supporting custom/unsupported targets (like x86_64-unknown-uefi).

I do think that this is a real problem, and that this problem is worth solving.

I would prefer to move all the ctype definitions into a separate crate, and for libc to depend on it. That would allow people that only want to use ctypes to not have to depend on libc. That crate could have a "generic" cargo feature that, for unsupported platforms, provides generic ctypes instead of providing nothing, and libc could re-export that feature (or not, just add a target-dependent dependency on ctypes instead of libc for custom targets).

Still, if the plan here is for ring to use those ctypes with the bundled libc, that approach wouldn't work unless the bundled libc was re-compiled with that generic feature.

[josephlr]

@gnzlbg your concerns make sense. I'll refactor this to use a generic_ctypes cargo flag.

types like int64_t are optional in C, that is, they are not guaranteed to be defined for a target.

I would think about this a little differently, while there are some (extremely obscure) platforms that do not support C's int64_t, this crate necessarily only needs to think about Rust targets, and the Rust spec requires an i64 type. So if this crate can be compiled, int64_t exists and is equal to i64.

intptr_t and size_t don't necessarily need to have the same bit-width (e.g. on CHARI intptr_t is 128-bit wide but size_t is only 64-bit wide, etc.).

This is interesting (I couldn't find anything online about CHARI). I think it means that there are three fundamental categories of mandatory C types at play here.

1. Types that have their value fixed by the union of the C and Rust standards
   - schar (i8), uchar (u8), int{8,116,32,64}_t (i{8,116,32,64}), uint{8,116,32,64}_t (u{8,116,32,64}), intptr_t (isize), uintptr_t (usize), float (f32), double (f64)
2. Types that can technically have different values, but fixed in practice
   - int (i32), uint (u32), short (i16), ushort (u16), longlong, ptrdiff_t, ssize_t (isize), size_t, ulonglong, uintmax (usize)
3. Types that actually vary from system to system (but have standard reasonable definitions)
   - char (u8 or i8), long(i32 or i64), ulong (u32 or u64), wchar (u32 or i32)

I'll change this PR to have (1) defined unambiguously in the libc root, and put (2) and (3) behind the generic_ctypes flag. Does that sound reasonable?

I would prefer to move all the ctype definitions into a separate crate, and for libc to depend on it. That would allow people that only want to use ctypes to not have to depend on libc. That crate could have a "generic" cargo feature that, for unsupported platforms, provides generic ctypes instead of providing nothing, and libc could re-export that feature (or not, just add a target-dependent dependency on ctypes instead of libc for custom targets).

This seems like the best ultimate solution. ring only uses C types (to allow for linking against BoringSSL's C code). How would this actually work, would ctypes still be part of this repo? I'm happy to help with getting this working, I just don't know how to do it.

[gnzlbg]

How would this actually work, would ctypes still be part of this repo?

There is a crate called cty that already does this more or less. So ideally we would make that conform to our needs, move it into this repo, and add it as a dependency (libc would then just re-export the contents of this crate). In the mean time ring can actually just depend on that crate AFAICT (no need to depend on libc). We'll have to work around some kinds of allowing cty to be usable from libcore and libstd though.

[gnzlbg]

I'll change this PR to have (1) defined unambiguously in the libc root, and put (2) and (3) behind the generic_ctypes flag. Does that sound reasonable?

That sounds reasonable, it delivers value now, and it is forward compatible with re-exporting the types from the cty crate in the future.

[josephlr]

@gnzlbg please take a look. Is this what you were looking for?

[gnzlbg]

Looks good. Would you mind removing the rustfmt commit ? As you might have discovered, libc does not support that yet. I prefer to do that as part of a clean up PR afterwards.

[gnzlbg]

cc @japaric , @alexcrichton

Also see #375 , which pretty much was closed with the decision to leave libc empty for unsupported platforms, and the recommendation that platforms that only want to use generic ctypes (because that's the only thing they need, or because there is no libc for that platform) should use the cty crate.

This PR is different from what was discussed on that issue in that it only exposes these "generic" types behind a feature flag, instead of unconditionally. Still, once we land this, removing the feature and the types exposed would be a breaking change, so we should be sure that this is the direction in which we want to go. I'm still unconvinced of the advantage of doing this here instead of the cty crate. I wish we could mark a cargo feature as "unstable" or something.

[gnzlbg]

Also cc @SimonSapin @sfackler

[josephlr]

Looks good. Would you mind removing the rustfmt commit ? As you might have discovered, libc does not support that yet. I prefer to do that as part of a clean up PR afterwards.

Done, and ya I figured it out when I couldn't just run cargo fmt to fix all of the too long lines.

Also see #375 , which pretty much was closed with the decision to leave libc empty for unsupported platforms, and the recommendation that platforms that only want to use generic ctypes (because that's the only thing they need, or because there is no libc for that platform) should use the cty crate.

This change (as opposed to using libc and cty)is useful for cases where libraries sometimes want system bindings and sometimes do not (depending on arch). Otherwise, they would need even more Cargo flags and conditional compilation.

Removing the feature and the types exposed would be a breaking change, so we should be sure that this is the direction in which we want to go.

Agreed, I think before we stabilize we should make sure that we are correctly getting the types for int, long, char, and wchar. We can get int from the target, but long and char are quite tricky. I'm not worried about any of the other types (in that I don't think clang or gcc support a ptrdiff_t that's not isize, but we should check).

[josephlr]

Also @gnzlbg 5c1a6b8 was already a breaking change to those targets, as it made libc empty on those targets.

[gnzlbg]

I know 😆 But that change addressed a real issue.

Every time we added a new target to Rust, all that libc functionality was instantaneously stabilized for that target, independently of whether it was correct, whether it made sense (e.g. whether the target actually had a system / platform to bind to), etc.

This was useful and convenient when the functionality was correct - add a new target to Rust, some minimal functionality is instantaneously available in libc automatically - but an issue when it wasn't: fixing the functionality after it had already been added was always a technical breaking change (although loosely speaking, we are allowed to do breaking changes that fix things that were already broken anyways).

I don't see a problem with this being opt-in for now, and I doubt we'll ever support targets for which these definitions don't apply. If that were to happen, we can always do a breaking change to fix those.

From a design pov, this solves a problem for targets "without libc", so to me the question that still needs to be resolved is whether we want to support those in libc as well, or whether as #375 mentioned, if these crates only need some generic ctypes for these targets, why aren't they using the cty crate instead ?

EDIT: I admit that adding a libc dependency unconditionally is more convenient than switching between libc and cty depending on the target. So there is value in supporting these in libc itself. Although with this PR one would still need to enable the "generic_ctypes" feature depending on the target, so ideally we would avoid that once we are sure that this is where we want to go.

[josephlr]

Every time we added a new target to Rust, all that libc functionality was instantaneously stabilized for that target, independently of whether it was correct, whether it made sense (e.g. whether the target actually had a system / platform to bind to), etc.

Ahhhhh... Ok I can definitely see why that would be a major pain.

One final question to address is if we should be exporting the category (1) types in the libc root unconditionally like discussed here: #1244 (comment) those definitions can never be wrong, so I think it's fine.

Although with this PR one would still need to enable the "generic_ctypes" feature depending on the target, so ideally we would avoid that once we are sure that this is where we want to go.

Actually the feature can be enabled unconditionally, it only does something on unknown platforms.

[alexcrichton]

Sorry there's a lot of discussion here and I haven't reviewed all of it just yet, I wanted to clarify the ctypes-in-libc point. We can add c types in libc at any time so long as there's a definition of what they should actually be. For example if there's a C compiler that defines these types, we match that. If there is no C compiler for a target and/or no definition of what the types could be, then we're not in the business of defining the types ourselves.

[gnzlbg]

If there is no C compiler for a target and/or no definition of what the types could be, then we're not in the business of defining the types ourselves.

The problem here is that we don't know the target, so we don't know if there is a C compiler for it, and the question is whether we should expose anything in this case.

[josephlr]

The problem here is that we don't know the target, so we don't know if there is a C compiler for it,

Right now all rustc Targets have to have a field called target_c_int_width which seems to imply that all Rust target's should have a supported C compiler (note that rustc never actually uses target_c_int_width).

Clang also supports <arch>-unknown-unknown triples, so we could just fall back to those implementations if we:

• Recognize the architecture
• Do not recognize the os/env

[bors]

☔ The latest upstream changes (presumably #1247) made this pull request unmergeable. Please resolve the merge conflicts.

[retep998]

ssize_t is not a standard C type. 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]

@josephlr thank you for the PR, I'm going to close this for the time being, since this discussions has been moved to the cty crate: japaric/cty#14 Feel free to chime in there with your thoughts.

Basically, the same open questions that we had here are being discussed there, and once we resolve them, we'll move the cty crate into this repo, and hopefully this issue will be resolved.

[gnzlbg]

I forgot. Really, thank you for working on this, and let me know if you'd like to work on moving the cty crate to this repo. This is something that had been in the pipeline of things to tackle for a long time, and this was exactly the right push that it needed to get things going.