Repository: rust-lang/rust-bindgen
Pull request #2808 (2024-04-17 14:54:16)

Simplify Rust to Clang target conversion
This change simplifies/generalizes the rust to clang target conversion and allows more targets to work. (e.g. `riscv32i-unknown-none-elf`)

Let me know if I'm missing something!

pvdrz: I like the spirit of this PR as the current code has been pretty much a "oh damn, this other target doesn't work, let me get some tape" kind of situation,

It also makes me think that we should have some dedicated type for the target triples. So instead of doing `target.starts_with("architecture") we can do `target.architecture == "architecture"` or something.

Maybe using https://docs.rs/target-lexicon would be a good idea. It is an extra dependency, but it doesn't have other dependencies and it is well maintained IMHO.

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 4fa9dbcf..dcf6240a 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -40,7 +40,6 @@ dependencies = [
  "rustc-hash",
  "shlex",
  "syn 2.0.18",
- "which",
 ]
 
 [[package]]
@@ -661,17 +660,6 @@ version = "0.11.0+wasi-snapshot-preview1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423"
 
-[[package]]
-name = "which"
-version = "4.4.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2441c784c52b289a054b7201fc93253e288f094e2f4be9058343127c4226a269"
-dependencies = [
- "either",
- "libc",
- "once_cell",
-]
-
 [[package]]
 name = "winapi"
 version = "0.3.9"
diff --git a/bindgen-cli/Cargo.toml b/bindgen-cli/Cargo.toml
index 24066c4c..40b4b4cb 100644
--- a/bindgen-cli/Cargo.toml
+++ b/bindgen-cli/Cargo.toml
@@ -28,12 +28,10 @@ log = { version = "0.4", optional = true }
 shlex = "1"
 
 [features]
-default = ["logging", "runtime", "which-rustfmt"]
+default = ["logging", "runtime"]
 logging = ["bindgen/logging", "dep:env_logger", "dep:log"]
 static = ["bindgen/static"]
 runtime = ["bindgen/runtime"]
-# Dynamically discover a `rustfmt` binary using the `which` crate
-which-rustfmt = ["bindgen/which-rustfmt"]
 prettyplease = ["bindgen/prettyplease"]
 
 ## The following features are for internal use and they shouldn't be used if
diff --git a/bindgen/Cargo.toml b/bindgen/Cargo.toml
index 95293daf..4e3ad81b 100644
--- a/bindgen/Cargo.toml
+++ b/bindgen/Cargo.toml
@@ -40,15 +40,14 @@ regex = { version = "1.5.3", default-features = false, features = ["std", "unico
 rustc-hash = "1.0.1"
 shlex = "1"
 syn = { version = "2.0", features = ["full", "extra-traits", "visit-mut"] }
-which = { version = "4.2.1", optional = true, default-features = false }
 
 [features]
-default = ["logging", "prettyplease", "runtime", "which-rustfmt"]
+default = ["logging", "prettyplease", "runtime"]
 logging = ["dep:log"]
 static = ["clang-sys/static"]
 runtime = ["clang-sys/runtime"]
-# Dynamically discover a `rustfmt` binary using the `which` crate
-which-rustfmt = ["dep:which"]
+# This feature is no longer used for anything and should be removed in bindgen 0.70
+which-rustfmt = []
 experimental = ["dep:annotate-snippets"]
 
 ## The following features are for internal use and they shouldn't be used if
diff --git a/bindgen/codegen/dyngen.rs b/bindgen/codegen/dyngen.rs
index b26c0b18..4b2749ec 100644
--- a/bindgen/codegen/dyngen.rs
+++ b/bindgen/codegen/dyngen.rs
@@ -1,7 +1,7 @@
 use crate::codegen;
 use crate::ir::context::BindgenContext;
 use crate::ir::function::ClangAbi;
-use proc_macro2::Ident;
+use proc_macro2::{Ident, TokenStream};
 
 /// Used to build the output tokens for dynamic bindings.
 #[derive(Default)]
@@ -122,7 +122,7 @@ impl DynamicItems {
     }
 
     #[allow(clippy::too_many_arguments)]
-    pub(crate) fn push(
+    pub(crate) fn push_func(
         &mut self,
         ident: Ident,
         abi: ClangAbi,
@@ -196,4 +196,47 @@ impl DynamicItems {
             #ident
         });
     }
+
+    pub fn push_var(
+        &mut self,
+        ident: Ident,
+        ty: TokenStream,
+        is_required: bool,
+    ) {
+        let member = if is_required {
+            quote! { *mut #ty }
+        } else {
+            quote! { Result<*mut #ty, ::libloading::Error> }
+        };
+
+        self.struct_members.push(quote! {
+            pub #ident: #member,
+        });
+
+        let deref = if is_required {
+            quote! { self.#ident }
+        } else {
+            quote! { *self.#ident.as_ref().expect("Expected variable, got error.") }
+        };
+        self.struct_implementation.push(quote! {
+            pub unsafe fn #ident (&self) -> *mut #ty {
+                #deref
+            }
+        });
+
+        let ident_str = codegen::helpers::ast_ty::cstr_expr(ident.to_string());
+        self.constructor_inits.push(if is_required {
+            quote! {
+                let #ident = __library.get::<*mut #ty>(#ident_str).map(|sym| *sym)?;
+            }
+        } else {
+            quote! {
+                let #ident = __library.get::<*mut #ty>(#ident_str).map(|sym| *sym);
+            }
+        });
+
+        self.init_fields.push(quote! {
+            #ident
+        });
+    }
 }
diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs
index 070d9dec..0770275d 100644
--- a/bindgen/codegen/mod.rs
+++ b/bindgen/codegen/mod.rs
@@ -53,7 +53,7 @@ use crate::ir::ty::{Type, TypeKind};
 use crate::ir::var::Var;
 
 use proc_macro2::{Ident, Span};
-use quote::TokenStreamExt;
+use quote::{ToTokens, TokenStreamExt};
 
 use crate::{Entry, HashMap, HashSet};
 use std::borrow::Cow;
@@ -799,7 +799,17 @@ impl CodeGenerator for Var {
                 }
             );
 
-            result.push(tokens);
+            if ctx.options().dynamic_library_name.is_some() {
+                result.dynamic_items().push_var(
+                    canonical_ident,
+                    self.ty()
+                        .to_rust_ty_or_opaque(ctx, &())
+                        .into_token_stream(),
+                    ctx.options().dynamic_link_require_all,
+                );
+            } else {
+                result.push(tokens);
+            }
         }
     }
 }
@@ -4576,7 +4586,7 @@ impl CodeGenerator for Function {
             let args_identifiers =
                 utils::fnsig_argument_identifiers(ctx, signature);
             let ret_ty = utils::fnsig_return_ty(ctx, signature);
-            result.dynamic_items().push(
+            result.dynamic_items().push_func(
                 ident,
                 abi,
                 signature.is_variadic(),
diff --git a/bindgen/ir/comp.rs b/bindgen/ir/comp.rs
index f6c9e629..13a8184f 100644
--- a/bindgen/ir/comp.rs
+++ b/bindgen/ir/comp.rs
@@ -1449,7 +1449,7 @@ impl CompInfo {
                 }
                 CXCursor_TemplateTypeParameter => {
                     let param = Item::type_param(None, cur, ctx).expect(
-                        "Item::type_param should't fail when pointing \
+                        "Item::type_param shouldn't fail when pointing \
                          at a TemplateTypeParameter",
                     );
                     ci.template_params.push(param);
diff --git a/bindgen/lib.rs b/bindgen/lib.rs
index c2df8a8a..8cc69f47 100644
--- a/bindgen/lib.rs
+++ b/bindgen/lib.rs
@@ -686,45 +686,31 @@ pub(crate) const HOST_TARGET: &str =
 // Some architecture triplets are different between rust and libclang, see #1211
 // and duplicates.
 fn rust_to_clang_target(rust_target: &str) -> Box<str> {
-    if rust_target.starts_with("aarch64-apple-") {
-        let mut clang_target = "arm64-apple-".to_owned();
-        clang_target
-            .push_str(rust_target.strip_prefix("aarch64-apple-").unwrap());
-        return clang_target.into();
-    } else if rust_target.starts_with("riscv64gc-") {
-        let mut clang_target = "riscv64-".to_owned();
-        clang_target.push_str(rust_target.strip_prefix("riscv64gc-").unwrap());
-        return clang_target.into();
-    } else if rust_target.starts_with("riscv64imac-") {
-        let mut clang_target = "riscv64-".to_owned();
-        clang_target
-            .push_str(rust_target.strip_prefix("riscv64imac-").unwrap());
-        return clang_target.into();
-    } else if rust_target.ends_with("-espidf") {
-        let mut clang_target =
-            rust_target.strip_suffix("-espidf").unwrap().to_owned();
-        clang_target.push_str("-elf");
-        if clang_target.starts_with("riscv32imc-") {
-            clang_target = "riscv32-".to_owned() +
-                clang_target.strip_prefix("riscv32imc-").unwrap();
-        }
-        return clang_target.into();
-    } else if rust_target.starts_with("riscv32imc-") {
-        let mut clang_target = "riscv32-".to_owned();
-        clang_target.push_str(rust_target.strip_prefix("riscv32imc-").unwrap());
-        return clang_target.into();
-    } else if rust_target.starts_with("riscv32imac-") {
-        let mut clang_target = "riscv32-".to_owned();
-        clang_target
-            .push_str(rust_target.strip_prefix("riscv32imac-").unwrap());
-        return clang_target.into();
-    } else if rust_target.starts_with("riscv32imafc-") {
-        let mut clang_target = "riscv32-".to_owned();
-        clang_target
-            .push_str(rust_target.strip_prefix("riscv32imafc-").unwrap());
-        return clang_target.into();
+    const TRIPLE_HYPHENS_MESSAGE: &str = "Target triple should contain hyphens";
+
+    let mut clang_target = rust_target.to_owned();
+
+    if clang_target.starts_with("riscv32") {
+        let idx = clang_target.find('-').expect(TRIPLE_HYPHENS_MESSAGE);
+
+        clang_target.replace_range(..idx, "riscv32");
+    } else if clang_target.starts_with("riscv64") {
+        let idx = clang_target.find('-').expect(TRIPLE_HYPHENS_MESSAGE);
+
+        clang_target.replace_range(..idx, "riscv64");
+    } else if clang_target.starts_with("aarch64-apple-") {
+        let idx = clang_target.find('-').expect(TRIPLE_HYPHENS_MESSAGE);
+
+        clang_target.replace_range(..idx, "arm64");
     }
-    rust_target.into()
+
+    if clang_target.ends_with("-espidf") {
+        let idx = clang_target.rfind('-').expect(TRIPLE_HYPHENS_MESSAGE);
+
+        clang_target.replace_range((idx + 1).., "elf");
+    }
+
+    clang_target.into()
 }
 
 /// Returns the effective target, and whether it was explicitly specified on the
@@ -983,14 +969,6 @@ impl Bindings {
         if let Ok(rustfmt) = env::var("RUSTFMT") {
             return Ok(Cow::Owned(rustfmt.into()));
         }
-        #[cfg(feature = "which-rustfmt")]
-        match which::which("rustfmt") {
-            Ok(p) => Ok(Cow::Owned(p)),
-            Err(e) => {
-                Err(io::Error::new(io::ErrorKind::Other, format!("{}", e)))
-            }
-        }
-        #[cfg(not(feature = "which-rustfmt"))]
         // No rustfmt binary was specified, so assume that the binary is called
         // "rustfmt" and that it is in the user's PATH.
         Ok(Cow::Owned("rustfmt".into()))
@@ -1383,6 +1361,10 @@ fn test_rust_to_clang_target_riscv() {
         rust_to_clang_target("riscv32imafc-unknown-none-elf").as_ref(),
         "riscv32-unknown-none-elf"
     );
+    assert_eq!(
+        rust_to_clang_target("riscv32i-unknown-none-elf").as_ref(),
+        "riscv32-unknown-none-elf"
+    );
 }
 
 #[test]


Test patch:
diff --git a/bindgen-tests/Cargo.toml b/bindgen-tests/Cargo.toml
index 1c06c3f1..a253b349 100644
--- a/bindgen-tests/Cargo.toml
+++ b/bindgen-tests/Cargo.toml
@@ -19,7 +19,6 @@ owo-colors = "3.5.0"
 logging = ["bindgen/logging"]
 static = ["bindgen/static"]
 runtime = ["bindgen/runtime"]
-which-rustfmt = ["bindgen/which-rustfmt"]
 
 __testing_only_extra_assertions = ["bindgen/__testing_only_extra_assertions"]
 __testing_only_libclang_9 = ["bindgen/__testing_only_libclang_9"]
diff --git a/bindgen-tests/tests/expectations/tests/dynamic_loading_variable_required.rs b/bindgen-tests/tests/expectations/tests/dynamic_loading_variable_required.rs
new file mode 100644
index 00000000..a96efbe5
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/dynamic_loading_variable_required.rs
@@ -0,0 +1,32 @@
+#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+pub struct TestLib {
+    __library: ::libloading::Library,
+    pub foo: *mut ::std::os::raw::c_int,
+    pub baz: *mut *mut ::std::os::raw::c_int,
+}
+impl TestLib {
+    pub unsafe fn new<P>(path: P) -> Result<Self, ::libloading::Error>
+    where
+        P: AsRef<::std::ffi::OsStr>,
+    {
+        let library = ::libloading::Library::new(path)?;
+        Self::from_library(library)
+    }
+    pub unsafe fn from_library<L>(library: L) -> Result<Self, ::libloading::Error>
+    where
+        L: Into<::libloading::Library>,
+    {
+        let __library = library.into();
+        let foo = __library.get::<*mut ::std::os::raw::c_int>(b"foo\0").map(|sym| *sym)?;
+        let baz = __library
+            .get::<*mut *mut ::std::os::raw::c_int>(b"baz\0")
+            .map(|sym| *sym)?;
+        Ok(TestLib { __library, foo, baz })
+    }
+    pub unsafe fn foo(&self) -> *mut ::std::os::raw::c_int {
+        self.foo
+    }
+    pub unsafe fn baz(&self) -> *mut *mut ::std::os::raw::c_int {
+        self.baz
+    }
+}
diff --git a/bindgen-tests/tests/expectations/tests/dynamic_loading_variable_simple.rs b/bindgen-tests/tests/expectations/tests/dynamic_loading_variable_simple.rs
new file mode 100644
index 00000000..ced70dbd
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/dynamic_loading_variable_simple.rs
@@ -0,0 +1,32 @@
+#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+pub struct TestLib {
+    __library: ::libloading::Library,
+    pub foo: Result<*mut ::std::os::raw::c_int, ::libloading::Error>,
+    pub baz: Result<*mut *mut ::std::os::raw::c_int, ::libloading::Error>,
+}
+impl TestLib {
+    pub unsafe fn new<P>(path: P) -> Result<Self, ::libloading::Error>
+    where
+        P: AsRef<::std::ffi::OsStr>,
+    {
+        let library = ::libloading::Library::new(path)?;
+        Self::from_library(library)
+    }
+    pub unsafe fn from_library<L>(library: L) -> Result<Self, ::libloading::Error>
+    where
+        L: Into<::libloading::Library>,
+    {
+        let __library = library.into();
+        let foo = __library.get::<*mut ::std::os::raw::c_int>(b"foo\0").map(|sym| *sym);
+        let baz = __library
+            .get::<*mut *mut ::std::os::raw::c_int>(b"baz\0")
+            .map(|sym| *sym);
+        Ok(TestLib { __library, foo, baz })
+    }
+    pub unsafe fn foo(&self) -> *mut ::std::os::raw::c_int {
+        *self.foo.as_ref().expect("Expected variable, got error.")
+    }
+    pub unsafe fn baz(&self) -> *mut *mut ::std::os::raw::c_int {
+        *self.baz.as_ref().expect("Expected variable, got error.")
+    }
+}
diff --git a/bindgen-tests/tests/expectations/tests/dynamic_loading_variable_with_allowlist.rs b/bindgen-tests/tests/expectations/tests/dynamic_loading_variable_with_allowlist.rs
new file mode 100644
index 00000000..3f29e738
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/dynamic_loading_variable_with_allowlist.rs
@@ -0,0 +1,30 @@
+#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+pub struct TestLib {
+    __library: ::libloading::Library,
+    pub foo: Result<*mut ::std::os::raw::c_int, ::libloading::Error>,
+    pub bar: Result<*mut ::std::os::raw::c_int, ::libloading::Error>,
+}
+impl TestLib {
+    pub unsafe fn new<P>(path: P) -> Result<Self, ::libloading::Error>
+    where
+        P: AsRef<::std::ffi::OsStr>,
+    {
+        let library = ::libloading::Library::new(path)?;
+        Self::from_library(library)
+    }
+    pub unsafe fn from_library<L>(library: L) -> Result<Self, ::libloading::Error>
+    where
+        L: Into<::libloading::Library>,
+    {
+        let __library = library.into();
+        let foo = __library.get::<*mut ::std::os::raw::c_int>(b"foo\0").map(|sym| *sym);
+        let bar = __library.get::<*mut ::std::os::raw::c_int>(b"bar\0").map(|sym| *sym);
+        Ok(TestLib { __library, foo, bar })
+    }
+    pub unsafe fn foo(&self) -> *mut ::std::os::raw::c_int {
+        *self.foo.as_ref().expect("Expected variable, got error.")
+    }
+    pub unsafe fn bar(&self) -> *mut ::std::os::raw::c_int {
+        *self.bar.as_ref().expect("Expected variable, got error.")
+    }
+}
diff --git a/bindgen-tests/tests/headers/constructors.hpp b/bindgen-tests/tests/headers/constructors.hpp
index d4174889..2f8d57b0 100644
--- a/bindgen-tests/tests/headers/constructors.hpp
+++ b/bindgen-tests/tests/headers/constructors.hpp
@@ -1,6 +1,6 @@
 
 class TestOverload {
-  // This one shouldnt' be generated.
+  // This one shouldn't be generated.
   TestOverload();
 public:
   TestOverload(int);
diff --git a/bindgen-tests/tests/headers/constructors_1_33.hpp b/bindgen-tests/tests/headers/constructors_1_33.hpp
index 7c6262d4..e275f890 100644
--- a/bindgen-tests/tests/headers/constructors_1_33.hpp
+++ b/bindgen-tests/tests/headers/constructors_1_33.hpp
@@ -1,7 +1,7 @@
 // bindgen-flags: --rust-target 1.33
 
 class TestOverload {
-  // This one shouldnt' be generated.
+  // This one shouldn't be generated.
   TestOverload();
 public:
   /// Calling this should use `mem::unintialized()` and not `MaybeUninit()` as only rust 1.36 includes that.
diff --git a/bindgen-tests/tests/headers/dynamic_loading_variable_required.h b/bindgen-tests/tests/headers/dynamic_loading_variable_required.h
new file mode 100644
index 00000000..8e811bb3
--- /dev/null
+++ b/bindgen-tests/tests/headers/dynamic_loading_variable_required.h
@@ -0,0 +1,4 @@
+// bindgen-flags: --dynamic-loading TestLib --dynamic-link-require-all
+
+int foo;
+int *baz;
\ No newline at end of file
diff --git a/bindgen-tests/tests/headers/dynamic_loading_variable_simple.h b/bindgen-tests/tests/headers/dynamic_loading_variable_simple.h
new file mode 100644
index 00000000..5ecad752
--- /dev/null
+++ b/bindgen-tests/tests/headers/dynamic_loading_variable_simple.h
@@ -0,0 +1,4 @@
+// bindgen-flags: --dynamic-loading TestLib
+
+int foo;
+int *baz;
\ No newline at end of file
diff --git a/bindgen-tests/tests/headers/dynamic_loading_variable_with_allowlist.hpp b/bindgen-tests/tests/headers/dynamic_loading_variable_with_allowlist.hpp
new file mode 100644
index 00000000..5e273030
--- /dev/null
+++ b/bindgen-tests/tests/headers/dynamic_loading_variable_with_allowlist.hpp
@@ -0,0 +1,5 @@
+// bindgen-flags: --dynamic-loading TestLib --allowlist-var foo --allowlist-var bar
+
+int foo;
+int bar;
+int baz; // should not be allowed
\ No newline at end of file