The Wayback Machine - https://web.archive.org/web/20201127075412/https://github.com/rust-lang/rust-bindgen/issues/1901
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

size_t_is_usize should default to true #1901

Open
dkg opened this issue Oct 16, 2020 · 2 comments · May be fixed by #1902
Open

size_t_is_usize should default to true #1901

dkg opened this issue Oct 16, 2020 · 2 comments · May be fixed by #1902

Comments

@dkg
Copy link

@dkg dkg commented Oct 16, 2020

In #1671, there is a reasonable argument made that a principled mapping between size_t and usize is 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 foo exposes a bindgen-derived mapping of a C function size_t bar(), then bar will have a different signature on x86 (where it returns a u32) than on x86_64 (where it returns a u64). 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_usize defaults to true), then if bindgen is handling any size_t on a platform where usizesize_t it could fail and abort. A package that wants to can still set size_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).

@dkg dkg mentioned this issue Oct 16, 2020
@dkg
Copy link
Author

@dkg dkg commented Oct 16, 2020

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 size_t_is_usize(true) and exposing a size_t somewhere. but i think that risk of API change is worthwhile for the convergent API across platforms (and for keeping the generated API stable from versions of bindgen before 0.53 as well).

dkg added a commit to dkg/rust-bindgen that referenced this issue Oct 16, 2020
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.
@dkg dkg linked a pull request that will close this issue Oct 16, 2020
dkg added a commit to dkg/rust-bindgen that referenced this issue Oct 16, 2020
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
dkg added a commit to dkg/rust-bindgen that referenced this issue Oct 16, 2020
@dkg
Copy link
Author

@dkg dkg commented Oct 16, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant
You can’t perform that action at this time.