Skip to content

Commit

Permalink
Map size_t to usize by default and check compatibility (fixes rust-la…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
geofft authored and pvdrz committed Sep 24, 2022
1 parent 6de2d3d commit cc78b6f
Show file tree
Hide file tree
Showing 14 changed files with 81 additions and 31 deletions.
31 changes: 31 additions & 0 deletions asd.fish
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
for name in (rg -lF addr_of! tests/expectations | sd '.*/(.*).rs' '$1')
set path (fd --glob "$name.*" tests/headers)
if test -n "$path"

set flags (rg -F "// bindgen-flags" $path)
if test -n "$flags"
set minor (rg ".*\-\-rust\-target[ =]1.(\d+).*" $path -r '$1')
if test -n "$minor"
if test $minor -gt 47
echo $path needs to change the version from 1.$minor to 1.47
sd -s "1.$minor" "1.47" $path
else
echo $path uses version 1.$minor and that is fine
end
else
echo $path does not have the `--rust-target` flag
sd "// bindgen-flags: (.*)" '// bindgen-flags: --rust-target 1.47 $1' $path
end
else
echo $path does not have the flags at all
set contents (echo -e "// bindgen-flags: --rust-target 1.47\n"; cat $path)
rm $path
touch $path
for line in $contents
echo $line >> $path
end
end
else
echo $name headers not found
end
end
27 changes: 26 additions & 1 deletion src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -836,9 +836,34 @@ impl CodeGenerator for Type {
}

// If this is a known named type, disallow generating anything
// for it too.
// for it too. If size_t -> usize conversions are enabled, we
// need to check that these conversions are permissible, but
// nothing needs to be generated, still.
let spelling = self.name().expect("Unnamed alias?");
if utils::type_from_named(ctx, spelling).is_some() {
if let "size_t" | "ssize_t" = spelling {
let layout = inner_item
.kind()
.expect_type()
.layout(ctx)
.expect("No layout?");
assert_eq!(
layout.size,
ctx.target_pointer_size(),
"Target platform requires `--no-size_t-is-usize`. The size of `{}` ({}) does not match the target pointer size ({})",
spelling,
layout.size,
ctx.target_pointer_size(),
);
assert_eq!(
layout.align,
ctx.target_pointer_size(),
"Target platform requires `--no-size_t-is-usize`. The alignment of `{}` ({}) does not match the target pointer size ({})",
spelling,
layout.align,
ctx.target_pointer_size(),
);
}
return;
}

Expand Down
6 changes: 3 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,8 +564,8 @@ impl Builder {
output_vector.push("--no-record-matches".into());
}

if self.options.size_t_is_usize {
output_vector.push("--size_t-is-usize".into());
if !self.options.size_t_is_usize {
output_vector.push("--no-size_t-is-usize".into());
}

if !self.options.rustfmt_bindings {
Expand Down Expand Up @@ -2253,7 +2253,7 @@ impl Default for BindgenOptions {
time_phases: false,
record_matches: true,
rustfmt_bindings: true,
size_t_is_usize: false,
size_t_is_usize: true,
rustfmt_configuration_file: None,
no_partialeq_types: Default::default(),
no_copy_types: Default::default(),
Expand Down
11 changes: 8 additions & 3 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,12 @@ where
),
Arg::new("size_t-is-usize")
.long("size_t-is-usize")
.help("Translate size_t to usize."),
.help("Ignored - this is enabled by default.")
.hidden(true),
Arg::with_name("no-size_t-is-usize")
.long("no-size_t-is-usize")
.help("Do not bind size_t as usize (useful on platforms \
where those types are incompatible)."),
Arg::new("no-rustfmt-bindings")
.long("no-rustfmt-bindings")
.help("Do not format the generated bindings with rustfmt."),
Expand Down Expand Up @@ -975,8 +980,8 @@ where
builder = builder.record_matches(false);
}

if matches.is_present("size_t-is-usize") {
builder = builder.size_t_is_usize(true);
if matches.is_present("no-size_t-is-usize") {
builder = builder.size_t_is_usize(false);
}

let no_rustfmt_bindings = matches.is_present("no-rustfmt-bindings");
Expand Down
12 changes: 3 additions & 9 deletions tests/expectations/tests/blocks-signature.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion tests/expectations/tests/blocks.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions tests/expectations/tests/issue-1498.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions tests/expectations/tests/jsval_layout_opaque.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions tests/expectations/tests/jsval_layout_opaque_1_0.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions tests/expectations/tests/layout_array.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions tests/expectations/tests/msvc-no-usr.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion tests/expectations/tests/nsBaseHashtable.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// bindgen-flags: --size_t-is-usize
// bindgen-flags: --no-size_t-is-usize

typedef unsigned long size_t;
typedef long ssize_t;
Expand Down

0 comments on commit cc78b6f

Please sign in to comment.