size_t_is_usize should default to true #1901
Comments
|
If bindgen does make this change, then it'll cause another API change in downstream projects that were already using bindgen between 0.53 and 0.55 that have not explicitly set |
This addresses the first part of rust-lang#1901. If size_t_is_usize is manually set to false, and bindgen encounters any size_t, then we should also probably test to confirm that size_t is congruent to uintptr_t on the current platform. I'm not sure of the best way to do this check.
If size_t_is_usize is manually set to false, and bindgen encounters any size_t, then we should also probably test to confirm that size_t is congruent to uintptr_t on the current platform. I'm not sure of the best way to do this check. Fixes: rust-lang#1901
Fixes: rust-lang#1901 (see also: rust-lang#1903)
|
I've separated out the idea of doing a check (failing with an error) as a different ticket (#1903), so that this ticket can focus specifically on changing the default. |
|
👍 To make sure I'm understanding, and to clarify why I think this is important: The previous behavior allowed me as a crate author to pre-build bindings and include them with my crate. This let's my users use my crate without having to compile the bindgen dependency tree, which necessarily is very large and slow to build. Skipping this is a real improvement for my users, who, like all rust users would love to speed up their builds. Maybe packaging prebuilt bindings is controversial - but it's far from niche:
Typically (as with these crates) a It does seem like though One other thing I thought was interesting - seemingly the |
|
I wonder if the ultimate solution is to try to get c_size_t added to std::os::raw 😬 |
Our crate, nettle-sys, does not pre-build bindings, and we are also negatively affected by this change. From my point of view, these are the negative effects of this change:
|
…ng#1901, rust-lang#1903) This addresses the underlying issue identified in rust-lang#1671, that size_t (integer that can hold any object size) isn't guaranteed to match usize, which is defined more like uintptr_t (integer that can hold any pointer). However, on almost all platforms, this is true, and in fact Rust already uses usize extensively in contexts where size_t would be more appropriate, such as slice indexing. So, it's better for ergonomics when interfacing with C code to map the C size_t type to usize. (See also discussion in rust-lang/rust#65473 about how usize really should be defined as size_t, not uintptr_t.) The previous fix for rust-lang#1671 removed the special case for size_t and defaulted to binding it as a normal typedef. This change effectively reverts that and goes back to mapping size_t to usize (and ssize_t to isize), but also ensures that if size_t is emitted, the typedef'd type of size_t in fact is compatible with usize (defined by checking that the size and alignment match the target pointer width). For (hypothetical) platforms where this is not true, or for compatibility with the default behavior of bindgen between 0.53 and this commit, onwards, you can disable this mapping with --no-size_t-is-usize.
|
Indeed, the change associated with #1671 breaks builds of my thesis work under newer versions of bindgen. I can work around the issue with |
|
This behaviour (the type being I acknowledge the conflicting requirements but this does feel a lot worse than making the cheeky assumption or assertion that I don't think the library author stood much of a chance of noticing this and it's not clear to me that the users of a crate should be having to suspect that the type annotations of their dependencies will change per-platform.
This sounds sensible to me, though in the meantime, wonder if this could be done with an external crate which defines |
This will give us identical bindings for all platforms regardless of what underlying C type they use to define size_t. For some reason, this isn't the default: rust-lang/rust-bindgen#1901 Previously, we generated different things on different platforms: x86-64: pub type size_t = ::std::os::raw::c_ulong; 32-bit arm: pub type size_t = ::std::os::raw::c_uint; It also lets us eliminate a couple of size_t casts in the Rust wrapper code. Bug: None Test: (cd rust/minijail; cargo test) Change-Id: Ic37fc7fb2cf2ad173b4bc184a499011d299c8917
…ng#1901, rust-lang#1903) This addresses the underlying issue identified in rust-lang#1671, that size_t (integer that can hold any object size) isn't guaranteed to match usize, which is defined more like uintptr_t (integer that can hold any pointer). However, on almost all platforms, this is true, and in fact Rust already uses usize extensively in contexts where size_t would be more appropriate, such as slice indexing. So, it's better for ergonomics when interfacing with C code to map the C size_t type to usize. (See also discussion in rust-lang/rust#65473 about how usize really should be defined as size_t, not uintptr_t.) The previous fix for rust-lang#1671 removed the special case for size_t and defaulted to binding it as a normal typedef. This change effectively reverts that and goes back to mapping size_t to usize (and ssize_t to isize), but also ensures that if size_t is emitted, the typedef'd type of size_t in fact is compatible with usize (defined by checking that the size and alignment match the target pointer width). For (hypothetical) platforms where this is not true, or for compatibility with the default behavior of bindgen between 0.53 and this commit, onwards, you can disable this mapping with --no-size_t-is-usize.
…ng#1901, rust-lang#1903) This addresses the underlying issue identified in rust-lang#1671, that size_t (integer that can hold any object size) isn't guaranteed to match usize, which is defined more like uintptr_t (integer that can hold any pointer). However, on almost all platforms, this is true, and in fact Rust already uses usize extensively in contexts where size_t would be more appropriate, such as slice indexing. So, it's better for ergonomics when interfacing with C code to map the C size_t type to usize. (See also discussion in rust-lang/rust#65473 about how usize really should be defined as size_t, not uintptr_t.) The previous fix for rust-lang#1671 removed the special case for size_t and defaulted to binding it as a normal typedef. This change effectively reverts that and goes back to mapping size_t to usize (and ssize_t to isize), but also ensures that if size_t is emitted, the typedef'd type of size_t in fact is compatible with usize (defined by checking that the size and alignment match the target pointer width). For (hypothetical) platforms where this is not true, or for compatibility with the default behavior of bindgen between 0.53 and this commit, onwards, you can disable this mapping with --no-size_t-is-usize.
In #1671, there is a reasonable argument made that a principled mapping between
size_tandusizeis improper.However, all platforms that i'm aware of do equate the two.
bindgen's defaults since 0.53 mean that any binding that exposes a size_t results in an incompatible API across platforms. That is, if some package
fooexposes a bindgen-derived mapping of a C functionsize_t bar(), thenbarwill have a different signature on x86 (where it returns au32) than on x86_64 (where it returns au64). All downstream code that relies on that function will find itself dealing with the same divergent API across platforms. That's pretty weird and ugly for a typesafe langauge like rust. The behavior before 0.53 meant that the exposed API was congruent across platforms.I think bindgen's default should be what the pre-0.53 behavior was, even though it is not in principle "correct". If we do that (so that
size_t_is_usizedefaults totrue), then if bindgen is handling anysize_ton a platform whereusize≠size_tit could fail and abort. A package that wants to can still setsize_t_is_usize(false)if they want to be able to build on such a platform (and impose the consequent API type churn on their downstream dependencies).The text was updated successfully, but these errors were encountered: