Repository: rust-lang/rust-bindgen
Pull request #2471 (2023-03-26 18:38:26)

Generate `CStr`s instead of byte arrays.
~~The `bindgen` MSRV is 1.60.0, `CStr::from_bytes_with_nul` is `const`-stable since 1.59.0.~~

I see that the MSRV is only for compiling `bindgen`, so I added a new target for Rust 1.59 instead of always generating `CStr`s.

There is also a discussion regarding this in https://github.com/rust-lang/rust-bindgen/pull/2369#discussion_r1135967775.


bors-servo: :umbrella: The latest upstream changes (presumably 040149b097b2c460a73ed27539112dc5104994ad) made this pull request unmergeable. Please resolve the merge conflicts.
reitermarkus: Added a `--generate-cstr` option.
reitermarkus: > how the rest of the C strings are handled

What is the “rest”? Function signatures?

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index bbd4f198..393d266e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -166,7 +166,13 @@
 
 ## Added
 
+* Added the `--generate-cstr` CLI flag to generate string constants as `&CStr`
+  instead of `&[u8]`. (Requires Rust 1.59 or higher.)
+
 ## Changed
+
+* Non-UTF-8 string constants are now generated as references (`&[u8; SIZE]`)
+  instead of arrays (`[u8; SIZE]`) to match UTF-8 strings.
 * Wrappers for static functions that return `void` no longer contain a `return`
   statement and only call the static function instead.
    
@@ -252,7 +258,7 @@
  * The return type is now ommited in signatures of functions returning `void`.
  * Updated the `clap` dependency for `bindgen-cli` to 4.
  * Rewrote the `bindgen-cli` argument parser which could introduce unexpected
-   behavior changes. 
+   behavior changes.
  * The `ParseCallbacks::add_derives` method now receives `DeriveInfo<'_>` as
    argument instead of a `&str`. This type also includes the kind of target type.
 
diff --git a/bindgen-cli/options.rs b/bindgen-cli/options.rs
index 9ab202f1..8e292f95 100644
--- a/bindgen-cli/options.rs
+++ b/bindgen-cli/options.rs
@@ -156,6 +156,9 @@ struct BindgenCommand {
     /// Generate block signatures instead of void pointers.
     #[arg(long)]
     generate_block: bool,
+    /// Generate string constants as `&CStr` instead of `&[u8]`.
+    #[arg(long)]
+    generate_cstr: bool,
     /// Use extern crate instead of use for block.
     #[arg(long)]
     block_extern_crate: bool,
@@ -430,6 +433,7 @@ where
         no_recursive_allowlist,
         objc_extern_crate,
         generate_block,
+        generate_cstr,
         block_extern_crate,
         distrust_clang_mangling,
         builtins,
@@ -755,6 +759,10 @@ where
         builder = builder.generate_block(true);
     }
 
+    if generate_cstr {
+        builder = builder.generate_cstr(true);
+    }
+
     if block_extern_crate {
         builder = builder.block_extern_crate(true);
     }
diff --git a/bindgen/codegen/helpers.rs b/bindgen/codegen/helpers.rs
index 726fe75e..8231e07c 100644
--- a/bindgen/codegen/helpers.rs
+++ b/bindgen/codegen/helpers.rs
@@ -249,12 +249,6 @@ pub(crate) mod ast_ty {
         quote!(#val)
     }
 
-    pub(crate) fn byte_array_expr(bytes: &[u8]) -> TokenStream {
-        let mut bytes: Vec<_> = bytes.to_vec();
-        bytes.push(0);
-        quote! { [ #(#bytes),* ] }
-    }
-
     pub(crate) fn cstr_expr(mut string: String) -> TokenStream {
         string.push('\0');
         let b = proc_macro2::Literal::byte_string(string.as_bytes());
diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs
index 11e49b8d..81aa897e 100644
--- a/bindgen/codegen/mod.rs
+++ b/bindgen/codegen/mod.rs
@@ -57,9 +57,10 @@ use crate::{Entry, HashMap, HashSet};
 use std::borrow::Cow;
 use std::cell::Cell;
 use std::collections::VecDeque;
+use std::ffi::CStr;
 use std::fmt::{self, Write};
 use std::ops;
-use std::str::FromStr;
+use std::str::{self, FromStr};
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum CodegenError {
@@ -692,41 +693,48 @@ impl CodeGenerator for Var {
                     });
                 }
                 VarType::String(ref bytes) => {
-                    // Account the trailing zero.
-                    //
+                    let prefix = ctx.trait_prefix();
+
+                    let options = ctx.options();
+                    let rust_features = options.rust_features;
+
+                    let mut cstr_bytes = bytes.clone();
+                    cstr_bytes.push(0);
+                    let len = proc_macro2::Literal::usize_unsuffixed(
+                        cstr_bytes.len(),
+                    );
+                    let cstr = CStr::from_bytes_with_nul(&cstr_bytes).unwrap();
+
                     // TODO: Here we ignore the type we just made up, probably
                     // we should refactor how the variable type and ty ID work.
-                    let len = bytes.len() + 1;
-                    let ty = quote! {
-                        [u8; #len]
-                    };
+                    let array_ty = quote! { [u8; #len] };
+                    let cstr_ty = quote! { ::#prefix::ffi::CStr };
 
-                    match String::from_utf8(bytes.clone()) {
-                        Ok(string) => {
-                            let cstr = helpers::ast_ty::cstr_expr(string);
-                            if ctx
-                                .options()
-                                .rust_features
-                                .static_lifetime_elision
-                            {
-                                result.push(quote! {
-                                    #(#attrs)*
-                                    pub const #canonical_ident : &#ty = #cstr ;
-                                });
-                            } else {
-                                result.push(quote! {
-                                    #(#attrs)*
-                                    pub const #canonical_ident : &'static #ty = #cstr ;
-                                });
-                            }
-                        }
-                        Err(..) => {
-                            let bytes = helpers::ast_ty::byte_array_expr(bytes);
-                            result.push(quote! {
-                                #(#attrs)*
-                                pub const #canonical_ident : #ty = #bytes ;
-                            });
+                    let bytes = proc_macro2::Literal::byte_string(
+                        cstr.to_bytes_with_nul(),
+                    );
+
+                    if rust_features.const_cstr && options.generate_cstr {
+                        result.push(quote! {
+                            #(#attrs)*
+                            #[allow(unsafe_code)]
+                            pub const #canonical_ident: &#cstr_ty = unsafe {
+                                #cstr_ty::from_bytes_with_nul_unchecked(#bytes)
+                            };
+                        });
+                    } else {
+                        let lifetime = if rust_features.static_lifetime_elision
+                        {
+                            None
+                        } else {
+                            Some(quote! { 'static })
                         }
+                        .into_iter();
+
+                        result.push(quote! {
+                            #(#attrs)*
+                            pub const #canonical_ident: &#(#lifetime )*#array_ty = #bytes ;
+                        });
                     }
                 }
                 VarType::Float(f) => {
diff --git a/bindgen/features.rs b/bindgen/features.rs
index fe6f4158..bee0e859 100644
--- a/bindgen/features.rs
+++ b/bindgen/features.rs
@@ -128,6 +128,9 @@ macro_rules! rust_target_base {
             /// Rust stable 1.47
             /// * `larger_arrays` ([Tracking issue](https://github.com/rust-lang/rust/pull/74060))
             => Stable_1_47 => 1.47;
+            /// Rust stable 1.59
+            /// * `CStr::from_bytes_with_nul_unchecked` in `const` contexts ([PR](https://github.com/rust-lang/rust/pull/54745))
+            => Stable_1_59 => 1.59;
             /// Rust stable 1.64
             ///  * `core_ffi_c` ([Tracking issue](https://github.com/rust-lang/rust/issues/94501))
             => Stable_1_64 => 1.64;
@@ -241,6 +244,9 @@ rust_feature_def!(
     Stable_1_47 {
         => larger_arrays;
     }
+    Stable_1_59 {
+        => const_cstr;
+    }
     Stable_1_64 {
         => core_ffi_c;
     }
diff --git a/bindgen/options/mod.rs b/bindgen/options/mod.rs
index c60da71e..83674e0e 100644
--- a/bindgen/options/mod.rs
+++ b/bindgen/options/mod.rs
@@ -1416,6 +1416,23 @@ options! {
         },
         as_args: "--generate-block",
     },
+    /// Whether to generate strings as `CStr`.
+    generate_cstr: bool {
+        methods: {
+            /// Set whether string constants should be generated as `&CStr` instead of `&[u8]`.
+            ///
+            /// A minimum Rust target of 1.59 is required for this to have any effect as support
+            /// for `CStr::from_bytes_with_nul_unchecked` in `const` contexts is needed.
+            ///
+            /// This option is disabled by default but will become enabled by default in a future
+            /// release, so enabling this is recommended.
+            pub fn generate_cstr(mut self, doit: bool) -> Self {
+                self.options.generate_cstr = doit;
+                self
+            }
+        },
+        as_args: "--generate-cstr",
+    },
     /// Whether to emit `#[macro_use] extern crate block;` instead of `use block;` in the prologue
     /// of the files generated from apple block files.
     block_extern_crate: bool {


Test patch:
diff --git a/bindgen-tests/tests/expectations/tests/libclang-5/constant-evaluate.rs b/bindgen-tests/tests/expectations/tests/libclang-5/constant-evaluate.rs
index 9debe39d..6263faed 100644
--- a/bindgen-tests/tests/expectations/tests/libclang-5/constant-evaluate.rs
+++ b/bindgen-tests/tests/expectations/tests/libclang-5/constant-evaluate.rs
@@ -21,5 +21,5 @@ pub const BAZ: ::std::os::raw::c_longlong = 24;
 pub const fuzz: f64 = 51.0;
 pub const BAZZ: ::std::os::raw::c_char = 53;
 pub const WAT: ::std::os::raw::c_char = 0;
-pub const bytestring: &[u8; 4usize] = b"Foo\0";
-pub const NOT_UTF8: [u8; 5usize] = [240u8, 40u8, 140u8, 40u8, 0u8];
+pub const bytestring: &[u8; 4] = b"Foo\0";
+pub const NOT_UTF8: &[u8; 5] = b"\xF0(\x8C(\0";
diff --git a/bindgen-tests/tests/expectations/tests/libclang-9/constant-evaluate.rs b/bindgen-tests/tests/expectations/tests/libclang-9/constant-evaluate.rs
index 9debe39d..6263faed 100644
--- a/bindgen-tests/tests/expectations/tests/libclang-9/constant-evaluate.rs
+++ b/bindgen-tests/tests/expectations/tests/libclang-9/constant-evaluate.rs
@@ -21,5 +21,5 @@ pub const BAZ: ::std::os::raw::c_longlong = 24;
 pub const fuzz: f64 = 51.0;
 pub const BAZZ: ::std::os::raw::c_char = 53;
 pub const WAT: ::std::os::raw::c_char = 0;
-pub const bytestring: &[u8; 4usize] = b"Foo\0";
-pub const NOT_UTF8: [u8; 5usize] = [240u8, 40u8, 140u8, 40u8, 0u8];
+pub const bytestring: &[u8; 4] = b"Foo\0";
+pub const NOT_UTF8: &[u8; 5] = b"\xF0(\x8C(\0";
diff --git a/bindgen-tests/tests/expectations/tests/macro_const.rs b/bindgen-tests/tests/expectations/tests/macro_const.rs
index de423a2a..eecd8bd7 100644
--- a/bindgen-tests/tests/expectations/tests/macro_const.rs
+++ b/bindgen-tests/tests/expectations/tests/macro_const.rs
@@ -5,10 +5,10 @@
     non_upper_case_globals
 )]
 
-pub const foo: &[u8; 4usize] = b"bar\0";
+pub const foo: &[u8; 4] = b"bar\0";
 pub const CHAR: u8 = 98u8;
 pub const CHARR: u8 = 0u8;
 pub const FLOAT: f64 = 5.09;
 pub const FLOAT_EXPR: f64 = 0.005;
 pub const LONG: u32 = 3;
-pub const INVALID_UTF8: [u8; 5usize] = [240u8, 40u8, 140u8, 40u8, 0u8];
+pub const INVALID_UTF8: &[u8; 5] = b"\xF0(\x8C(\0";
diff --git a/bindgen-tests/tests/expectations/tests/macro_const_1_0.rs b/bindgen-tests/tests/expectations/tests/macro_const_1_0.rs
index e1356611..8523a68a 100644
--- a/bindgen-tests/tests/expectations/tests/macro_const_1_0.rs
+++ b/bindgen-tests/tests/expectations/tests/macro_const_1_0.rs
@@ -5,10 +5,10 @@
     non_upper_case_globals
 )]
 
-pub const foo: &'static [u8; 4usize] = b"bar\0";
+pub const foo: &'static [u8; 4] = b"bar\0";
 pub const CHAR: u8 = 98u8;
 pub const CHARR: u8 = 0u8;
 pub const FLOAT: f64 = 5.09;
 pub const FLOAT_EXPR: f64 = 0.005;
 pub const LONG: u32 = 3;
-pub const INVALID_UTF8: [u8; 5usize] = [240u8, 40u8, 140u8, 40u8, 0u8];
+pub const INVALID_UTF8: &'static [u8; 5] = b"\xF0(\x8C(\0";
diff --git a/bindgen-tests/tests/expectations/tests/strings_array.rs b/bindgen-tests/tests/expectations/tests/strings_array.rs
new file mode 100644
index 00000000..76e8debd
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/strings_array.rs
@@ -0,0 +1,10 @@
+#![allow(
+    dead_code,
+    non_snake_case,
+    non_camel_case_types,
+    non_upper_case_globals
+)]
+
+pub const MY_STRING_UTF8: &'static [u8; 14] = b"Hello, world!\0";
+pub const MY_STRING_INTERIOR_NULL: &'static [u8; 7] = b"Hello,\0";
+pub const MY_STRING_NON_UTF8: &'static [u8; 7] = b"ABCDE\xFF\0";
diff --git a/bindgen-tests/tests/expectations/tests/strings_cstr.rs b/bindgen-tests/tests/expectations/tests/strings_cstr.rs
new file mode 100644
index 00000000..e7f275f3
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/strings_cstr.rs
@@ -0,0 +1,17 @@
+#![allow(
+    dead_code,
+    non_snake_case,
+    non_camel_case_types,
+    non_upper_case_globals
+)]
+
+#[allow(unsafe_code)]
+pub const MY_STRING_UTF8: &::std::ffi::CStr = unsafe {
+    ::std::ffi::CStr::from_bytes_with_nul_unchecked(b"Hello, world!\0")
+};
+#[allow(unsafe_code)]
+pub const MY_STRING_INTERIOR_NULL: &::std::ffi::CStr =
+    unsafe { ::std::ffi::CStr::from_bytes_with_nul_unchecked(b"Hello,\0") };
+#[allow(unsafe_code)]
+pub const MY_STRING_NON_UTF8: &::std::ffi::CStr =
+    unsafe { ::std::ffi::CStr::from_bytes_with_nul_unchecked(b"ABCDE\xFF\0") };
diff --git a/bindgen-tests/tests/headers/strings_array.h b/bindgen-tests/tests/headers/strings_array.h
new file mode 100644
index 00000000..6a61d710
--- /dev/null
+++ b/bindgen-tests/tests/headers/strings_array.h
@@ -0,0 +1,5 @@
+// bindgen-flags: --rust-target=1.0
+
+const char* MY_STRING_UTF8 = "Hello, world!";
+const char* MY_STRING_INTERIOR_NULL = "Hello,\0World!";
+const char* MY_STRING_NON_UTF8 = "ABCDE\xFF";
diff --git a/bindgen-tests/tests/headers/strings_cstr.h b/bindgen-tests/tests/headers/strings_cstr.h
new file mode 100644
index 00000000..8b1d40e6
--- /dev/null
+++ b/bindgen-tests/tests/headers/strings_cstr.h
@@ -0,0 +1,5 @@
+// bindgen-flags: --rust-target=1.59 --generate-cstr
+
+const char* MY_STRING_UTF8 = "Hello, world!";
+const char* MY_STRING_INTERIOR_NULL = "Hello,\0World!";
+const char* MY_STRING_NON_UTF8 = "ABCDE\xFF";