Repository: rust-lang/rust-bindgen
Pull request #2425 (2023-02-28 08:09:32)

Add `generated_link_name_override` method for `ParseCallbacks`.
`generated_link_name_override` lets the developer choose the link name for
a symbol.

If a link name is specified, the attribute `#[link_name = "\u{1}VALUE"]` will
be set, overriding any other potential link name, including the mangled name.

This commit also adds an option to the bindgen cli called `prefix-link-name`,
to prefix the link name of exported symbols.

I think this should properly solve https://github.com/rust-lang/rust-bindgen/issues/1375.


pvdrz: tests aren't passing. Do you have any idea why?
bors-servo: :umbrella: The latest upstream changes (presumably a8c8638d28f135823e913dab69b8a0d4fa4bbf15) made this pull request unmergeable. Please resolve the merge conflicts.
bors-servo: :umbrella: The latest upstream changes (presumably a1010634a11048a992edd95d88464e8b76c42ccc) made this pull request unmergeable. Please resolve the merge conflicts.
pvdrz: :wave: hey I checked your PR and I was wondering if the `--prefix-link-name` flag is worth it considering that it will be used to just remove the `\u{1}` prefix. Do you actually need to prefix something to the link name of every function?

Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 127e5c5e..4c3398d1 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -11,6 +11,8 @@ and introduce yourself.
 - [Code of Conduct](#code-of-conduct)
 - [Filing an Issue](#filing-an-issue)
 - [Looking to Start Contributing to `bindgen`?](#looking-to-start-contributing-to-bindgen)
+- [Prerequisites](#prerequisites)
+  - [`rustfmt` / `cargo fmt`](#rustfmt--cargo-fmt)
 - [Building](#building)
 - [Testing](#testing)
   - [Overview](#overview)
@@ -63,6 +65,16 @@ issue, provide us with:
 * [Issues labeled "help wanted"](https://github.com/rust-lang/rust-bindgen/labels/help%20wanted)
 * Still can't find something to work on? [Drop a comment here](https://github.com/rust-lang/rust-bindgen/issues/747)
 
+## Prerequisites
+
+### `rustfmt` / `cargo fmt`
+
+We use `nightly` channel for `rustfmt` so please set the appropriate setting your editor/IDE for that.
+
+For rust-analyzer, you can set `rustfmt.extraArgs = ['+nightly']`.
+
+To check via command line, you can run `cargo +nightly fmt --check`.
+
 ## Building
 
 To build the `bindgen` library and the `bindgen` executable:
diff --git a/bindgen-cli/options.rs b/bindgen-cli/options.rs
index 966e6aec..3f22e7ee 100644
--- a/bindgen-cli/options.rs
+++ b/bindgen-cli/options.rs
@@ -305,6 +305,9 @@ struct BindgenCommand {
     /// Require successful linkage to all functions in the library.
     #[arg(long)]
     dynamic_link_require_all: bool,
+    /// Prefix the name of exported symbols.
+    #[arg(long)]
+    prefix_link_name: Option<String>,
     /// Makes generated bindings `pub` only for items if the items are publically accessible in C++.
     #[arg(long)]
     respect_cxx_access_specs: bool,
@@ -462,6 +465,7 @@ where
         wasm_import_module_name,
         dynamic_loading,
         dynamic_link_require_all,
+        prefix_link_name,
         respect_cxx_access_specs,
         translate_enum_integer_types,
         c_naming,
@@ -868,6 +872,28 @@ where
         builder = builder.dynamic_link_require_all(true);
     }
 
+    if let Some(prefix_link_name) = prefix_link_name {
+        #[derive(Debug)]
+        struct PrefixLinkNameCallback {
+            prefix: String,
+        }
+
+        impl bindgen::callbacks::ParseCallbacks for PrefixLinkNameCallback {
+            fn generated_link_name_override(
+                &self,
+                item_info: bindgen::callbacks::ItemInfo<'_>,
+            ) -> Option<String> {
+                let mut prefix = self.prefix.clone();
+                prefix.push_str(item_info.name);
+                Some(prefix)
+            }
+        }
+
+        builder = builder.parse_callbacks(Box::new(PrefixLinkNameCallback {
+            prefix: prefix_link_name,
+        }))
+    }
+
     if respect_cxx_access_specs {
         builder = builder.respect_cxx_access_specs(true);
     }
diff --git a/bindgen/callbacks.rs b/bindgen/callbacks.rs
index b1339e46..92ed6f4e 100644
--- a/bindgen/callbacks.rs
+++ b/bindgen/callbacks.rs
@@ -45,6 +45,15 @@ pub trait ParseCallbacks: fmt::Debug {
         None
     }
 
+    /// This function will run for every extern variable and function. The returned value determines
+    /// the link name in the bindings.
+    fn generated_link_name_override(
+        &self,
+        _item_info: ItemInfo<'_>,
+    ) -> Option<String> {
+        None
+    }
+
     /// The integer kind an integer macro should have, given a name and the
     /// value of that macro, or `None` if you want the default to be chosen.
     fn int_macro(&self, _name: &str, _value: i64) -> Option<IntKind> {
diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs
index f4a64b35..35531e29 100644
--- a/bindgen/codegen/mod.rs
+++ b/bindgen/codegen/mod.rs
@@ -745,13 +745,18 @@ impl CodeGenerator for Var {
             }
         } else {
             // If necessary, apply a `#[link_name]` attribute
-            let link_name = self.mangled_name().unwrap_or_else(|| self.name());
-            if !utils::names_will_be_identical_after_mangling(
-                &canonical_name,
-                link_name,
-                None,
-            ) {
+            if let Some(link_name) = self.link_name() {
                 attrs.push(attributes::link_name::<false>(link_name));
+            } else {
+                let link_name =
+                    self.mangled_name().unwrap_or_else(|| self.name());
+                if !utils::names_will_be_identical_after_mangling(
+                    &canonical_name,
+                    link_name,
+                    None,
+                ) {
+                    attrs.push(attributes::link_name::<false>(link_name));
+                }
             }
 
             let maybe_mut = if self.is_const() {
@@ -4147,16 +4152,21 @@ impl CodeGenerator for Function {
         }
 
         let mut has_link_name_attr = false;
-        let link_name = mangled_name.unwrap_or(name);
-        if !is_dynamic_function &&
-            !utils::names_will_be_identical_after_mangling(
-                &canonical_name,
-                link_name,
-                Some(abi),
-            )
-        {
+        if let Some(link_name) = self.link_name() {
             attributes.push(attributes::link_name::<false>(link_name));
             has_link_name_attr = true;
+        } else {
+            let link_name = mangled_name.unwrap_or(name);
+            if !is_dynamic_function &&
+                !utils::names_will_be_identical_after_mangling(
+                    &canonical_name,
+                    link_name,
+                    Some(abi),
+                )
+            {
+                attributes.push(attributes::link_name::<false>(link_name));
+                has_link_name_attr = true;
+            }
         }
 
         // Unfortunately this can't piggyback on the `attributes` list because
diff --git a/bindgen/ir/function.rs b/bindgen/ir/function.rs
index bde53547..bb2303fa 100644
--- a/bindgen/ir/function.rs
+++ b/bindgen/ir/function.rs
@@ -82,6 +82,9 @@ pub(crate) struct Function {
     /// The mangled name, that is, the symbol.
     mangled_name: Option<String>,
 
+    /// The link name. If specified, overwrite mangled_name.
+    link_name: Option<String>,
+
     /// The ID pointing to the current function signature.
     signature: TypeId,
 
@@ -97,6 +100,7 @@ impl Function {
     pub(crate) fn new(
         name: String,
         mangled_name: Option<String>,
+        link_name: Option<String>,
         signature: TypeId,
         kind: FunctionKind,
         linkage: Linkage,
@@ -104,6 +108,7 @@ impl Function {
         Function {
             name,
             mangled_name,
+            link_name,
             signature,
             kind,
             linkage,
@@ -120,6 +125,11 @@ impl Function {
         self.mangled_name.as_deref()
     }
 
+    /// Get this function's link name.
+    pub fn link_name(&self) -> Option<&str> {
+        self.link_name.as_deref()
+    }
+
     /// Get this function's signature type.
     pub(crate) fn signature(&self) -> TypeId {
         self.signature
@@ -726,8 +736,21 @@ impl ClangSubItemParser for Function {
 
         let mangled_name = cursor_mangling(context, &cursor);
 
-        let function =
-            Self::new(name.clone(), mangled_name, sig, kind, linkage);
+        let link_name = context.options().last_callback(|callbacks| {
+            callbacks.generated_link_name_override(ItemInfo {
+                name: name.as_str(),
+                kind: ItemKind::Function,
+            })
+        });
+
+        let function = Self::new(
+            name.clone(),
+            mangled_name,
+            link_name,
+            sig,
+            kind,
+            linkage,
+        );
 
         Ok(ParseResult::New(function, Some(cursor)))
     }
diff --git a/bindgen/ir/var.rs b/bindgen/ir/var.rs
index 620b25cb..db20124c 100644
--- a/bindgen/ir/var.rs
+++ b/bindgen/ir/var.rs
@@ -37,6 +37,8 @@ pub(crate) struct Var {
     name: String,
     /// The mangled name of the variable.
     mangled_name: Option<String>,
+    /// The link name of the variable.
+    link_name: Option<String>,
     /// The type of the variable.
     ty: TypeId,
     /// The value of the variable, that needs to be suitable for `ty`.
@@ -50,6 +52,7 @@ impl Var {
     pub(crate) fn new(
         name: String,
         mangled_name: Option<String>,
+        link_name: Option<String>,
         ty: TypeId,
         val: Option<VarType>,
         is_const: bool,
@@ -58,6 +61,7 @@ impl Var {
         Var {
             name,
             mangled_name,
+            link_name,
             ty,
             val,
             is_const,
@@ -88,6 +92,11 @@ impl Var {
     pub(crate) fn mangled_name(&self) -> Option<&str> {
         self.mangled_name.as_deref()
     }
+
+    /// Get this variable's link name.
+    pub fn link_name(&self) -> Option<&str> {
+        self.link_name.as_deref()
+    }
 }
 
 impl DotAttributes for Var {
@@ -267,7 +276,7 @@ impl ClangSubItemParser for Var {
                 let ty = Item::builtin_type(type_kind, true, ctx);
 
                 Ok(ParseResult::New(
-                    Var::new(name, None, ty, Some(val), true),
+                    Var::new(name, None, None, ty, Some(val), true),
                     Some(cursor),
                 ))
             }
@@ -291,6 +300,13 @@ impl ClangSubItemParser for Var {
                     return Err(ParseError::Continue);
                 }
 
+                let link_name = ctx.options().last_callback(|callbacks| {
+                    callbacks.generated_link_name_override(ItemInfo {
+                        name: name.as_str(),
+                        kind: ItemKind::Var,
+                    })
+                });
+
                 let ty = cursor.cur_type();
 
                 // TODO(emilio): do we have to special-case constant arrays in
@@ -360,7 +376,8 @@ impl ClangSubItemParser for Var {
                 };
 
                 let mangling = cursor_mangling(ctx, &cursor);
-                let var = Var::new(name, mangling, ty, value, is_const);
+                let var =
+                    Var::new(name, mangling, link_name, ty, value, is_const);
 
                 Ok(ParseResult::New(var, Some(cursor)))
             }


Test patch:
diff --git a/bindgen-tests/tests/expectations/tests/prefix-link-name-c.rs b/bindgen-tests/tests/expectations/tests/prefix-link-name-c.rs
new file mode 100644
index 00000000..c363a787
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/prefix-link-name-c.rs
@@ -0,0 +1,11 @@
+#![allow(
+    dead_code,
+    non_snake_case,
+    non_camel_case_types,
+    non_upper_case_globals
+)]
+
+extern "C" {
+    #[link_name = "\u{1}foo_bar"]
+    pub fn bar() -> ::std::os::raw::c_int;
+}
diff --git a/bindgen-tests/tests/expectations/tests/prefix-link-name-cpp.rs b/bindgen-tests/tests/expectations/tests/prefix-link-name-cpp.rs
new file mode 100644
index 00000000..535fccbd
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/prefix-link-name-cpp.rs
@@ -0,0 +1,11 @@
+#![allow(
+    dead_code,
+    non_snake_case,
+    non_camel_case_types,
+    non_upper_case_globals
+)]
+
+extern "C" {
+    #[link_name = "\u{1}foo_foo"]
+    pub fn baz_foo() -> ::std::os::raw::c_int;
+}
diff --git a/bindgen-tests/tests/headers/prefix-link-name-c.h b/bindgen-tests/tests/headers/prefix-link-name-c.h
new file mode 100644
index 00000000..2432798f
--- /dev/null
+++ b/bindgen-tests/tests/headers/prefix-link-name-c.h
@@ -0,0 +1,4 @@
+// bindgen-parse-callbacks: prefix-link-name-foo_
+// bindgen-flags: --prefix-link-name foo_
+
+int bar(void);
diff --git a/bindgen-tests/tests/headers/prefix-link-name-cpp.hpp b/bindgen-tests/tests/headers/prefix-link-name-cpp.hpp
new file mode 100644
index 00000000..4fd26123
--- /dev/null
+++ b/bindgen-tests/tests/headers/prefix-link-name-cpp.hpp
@@ -0,0 +1,8 @@
+// bindgen-parse-callbacks: prefix-link-name-foo_
+// bindgen-flags: --prefix-link-name foo_
+
+namespace baz {
+
+int foo();
+
+} // end namespace baz
diff --git a/bindgen-tests/tests/parse_callbacks/mod.rs b/bindgen-tests/tests/parse_callbacks/mod.rs
index 00967fe8..85082b63 100644
--- a/bindgen-tests/tests/parse_callbacks/mod.rs
+++ b/bindgen-tests/tests/parse_callbacks/mod.rs
@@ -31,6 +31,30 @@ impl ParseCallbacks for RemovePrefixParseCallback {
     }
 }
 
+#[derive(Debug)]
+pub struct PrefixLinkNameParseCallback {
+    pub prefix: Option<String>,
+}
+
+impl PrefixLinkNameParseCallback {
+    pub fn new(prefix: &str) -> Self {
+        PrefixLinkNameParseCallback {
+            prefix: Some(prefix.to_string()),
+        }
+    }
+}
+
+impl ParseCallbacks for PrefixLinkNameParseCallback {
+    fn generated_link_name_override(
+        &self,
+        item_info: ItemInfo,
+    ) -> Option<String> {
+        self.prefix
+            .as_deref()
+            .map(|prefix| format!("{}{}", prefix, item_info.name))
+    }
+}
+
 #[derive(Debug)]
 struct EnumVariantRename;
 
@@ -76,6 +100,11 @@ pub fn lookup(cb: &str) -> Box<dyn ParseCallbacks> {
                     .to_owned();
                 let lnopc = RemovePrefixParseCallback::new(prefix.unwrap());
                 Box::new(lnopc)
+            } else if call_back.starts_with("prefix-link-name-") {
+                let prefix =
+                    call_back.split("prefix-link-name-").last().to_owned();
+                let plnpc = PrefixLinkNameParseCallback::new(prefix.unwrap());
+                Box::new(plnpc)
             } else {
                 panic!("Couldn't find name ParseCallbacks: {}", cb)
             }