Repository: rust-lang/rust-bindgen
Pull request #2351 (2022-11-16 22:38:53)

Extend `generated_name_override` callback to variables
## Context

Modifying (typically "prefixing") library symbols is needed to avoid conflicts for applications linking to multiple version of the same library, or when linking to several similar-but-different libraries. For example due to their histories, OpenSSL, BoringSSL and AWS-LC share many symbols in common. To avoid problems for downstream applications that depend on more than one of these libraries, symbol prefixing is needed.

## Problem

The recent addition of `ParseCallbacks::generated_name_override` allows for function names that were modified/prefixed during their build, to have the modifications undone so that the generated bindings expose the original/intended name of the function. This is working as intended. However, the problem is that this callback is currently limited to modifying only function names, whereas other symbols (i.e., "extern" variables) may likewise need their names modified/prefixed to avoid link conflicts.

## Change

* This change updates `ParseCallbacks::generated_name_override` to accept a second parameter indicating the kind of item the name applies to (currently, either `Function` or `Var`).
* A `CallbackItemKind` enum was added to serve as the type for this second parameter.
* Tests have been updated to verify that the names of both function and variable can be updated by this callback. 



pvdrz: @skmcgrail thanks for your contribution! I think it might be desirable to add more arguments to `generated_name_override` in the future and given that this is already a breaking change we could take an approach similar to this one https://github.com/rust-lang/rust-bindgen/pull/2355 and introduce a non-exhaustive struct (`ItemInfo`  might be a good name?). What do you think?
emilio: Yeah, while we're changing the API, let's do as @pvdrz suggests to make sure that we can extend it easily in the future. Thanks!

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7cac405a..c7519b62 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -154,6 +154,8 @@
 ## Added
 
 ## Changed
+ * Fixed name collisions when having a C `enum` and a `typedef` with the same
+   name.
 
 ## Removed
 
diff --git a/bindgen/callbacks.rs b/bindgen/callbacks.rs
index 5e8ac788..1e48a302 100644
--- a/bindgen/callbacks.rs
+++ b/bindgen/callbacks.rs
@@ -30,9 +30,9 @@ pub trait ParseCallbacks: fmt::Debug {
         MacroParsingBehavior::Default
     }
 
-    /// This function will run for every function. The returned value determines the name visible
-    /// in the bindings.
-    fn generated_name_override(&self, _function_name: &str) -> Option<String> {
+    /// This function will run for every extern variable and function. The returned value determines
+    /// the name visible in the bindings.
+    fn generated_name_override(&self, _item_info: ItemInfo) -> Option<String> {
         None
     }
 
@@ -122,3 +122,21 @@ pub struct DeriveInfo<'a> {
     /// The name of the type.
     pub name: &'a str,
 }
+
+/// An struct providing information about the item being passed to `ParseCallbacks::generated_name_override`.
+#[non_exhaustive]
+pub struct ItemInfo<'a> {
+    /// The name of the item
+    pub name: &'a str,
+    /// The kind of item
+    pub kind: ItemKind,
+}
+
+/// An enum indicating the kind of item for an ItemInfo.
+#[non_exhaustive]
+pub enum ItemKind {
+    /// A Function
+    Function,
+    /// A Variable
+    Var,
+}
diff --git a/bindgen/ir/function.rs b/bindgen/ir/function.rs
index ecb3202f..8e83d980 100644
--- a/bindgen/ir/function.rs
+++ b/bindgen/ir/function.rs
@@ -6,6 +6,7 @@ use super::dot::DotAttributes;
 use super::item::Item;
 use super::traversal::{EdgeKind, Trace, Tracer};
 use super::ty::TypeKind;
+use crate::callbacks::{ItemInfo, ItemKind};
 use crate::clang::{self, Attribute};
 use crate::parse::{ClangSubItemParser, ParseError, ParseResult};
 use clang_sys::{self, CXCallingConv};
@@ -712,10 +713,12 @@ impl ClangSubItemParser for Function {
             // but seems easy enough to handle it here.
             name.push_str("_destructor");
         }
-        if let Some(nm) = context
-            .options()
-            .last_callback(|callbacks| callbacks.generated_name_override(&name))
-        {
+        if let Some(nm) = context.options().last_callback(|callbacks| {
+            callbacks.generated_name_override(ItemInfo {
+                name: name.as_str(),
+                kind: ItemKind::Function,
+            })
+        }) {
             name = nm;
         }
         assert!(!name.is_empty(), "Empty function name.");
diff --git a/bindgen/ir/var.rs b/bindgen/ir/var.rs
index 610b9dfa..903e1ff5 100644
--- a/bindgen/ir/var.rs
+++ b/bindgen/ir/var.rs
@@ -7,7 +7,7 @@ use super::function::cursor_mangling;
 use super::int::IntKind;
 use super::item::Item;
 use super::ty::{FloatKind, TypeKind};
-use crate::callbacks::MacroParsingBehavior;
+use crate::callbacks::{ItemInfo, ItemKind, MacroParsingBehavior};
 use crate::clang;
 use crate::clang::ClangToken;
 use crate::parse::{ClangSubItemParser, ParseError, ParseResult};
@@ -272,7 +272,20 @@ impl ClangSubItemParser for Var {
                 ))
             }
             CXCursor_VarDecl => {
-                let name = cursor.spelling();
+                let mut name = cursor.spelling();
+                if cursor.linkage() == CXLinkage_External {
+                    if let Some(nm) = ctx.options().last_callback(|callbacks| {
+                        callbacks.generated_name_override(ItemInfo {
+                            name: name.as_str(),
+                            kind: ItemKind::Var,
+                        })
+                    }) {
+                        name = nm;
+                    }
+                }
+                // No more changes to name
+                let name = name;
+
                 if name.is_empty() {
                     warn!("Empty constant name?");
                     return Err(ParseError::Continue);


Test patch:
diff --git a/bindgen-tests/tests/expectations/tests/issue-1375-prefixed-functions.rs b/bindgen-tests/tests/expectations/tests/issue-1375-prefixed-functions.rs
index 835b7579..edcca46d 100644
--- a/bindgen-tests/tests/expectations/tests/issue-1375-prefixed-functions.rs
+++ b/bindgen-tests/tests/expectations/tests/issue-1375-prefixed-functions.rs
@@ -5,6 +5,14 @@
     non_upper_case_globals
 )]
 
+extern "C" {
+    #[link_name = "\u{1}my_custom_prefix_var_const_name"]
+    pub static var_const_name: ::std::os::raw::c_int;
+}
+extern "C" {
+    #[link_name = "\u{1}my_custom_prefix_var_mut_name"]
+    pub static mut var_mut_name: ::std::os::raw::c_int;
+}
 extern "C" {
     #[link_name = "\u{1}my_custom_prefix_function_name"]
     pub fn function_name(x: ::std::os::raw::c_int);
diff --git a/bindgen-tests/tests/headers/issue-1375-prefixed-functions.h b/bindgen-tests/tests/headers/issue-1375-prefixed-functions.h
index 4264e52d..cc37c8ad 100644
--- a/bindgen-tests/tests/headers/issue-1375-prefixed-functions.h
+++ b/bindgen-tests/tests/headers/issue-1375-prefixed-functions.h
@@ -1,4 +1,8 @@
 // bindgen-parse-callbacks: remove-function-prefix-my_custom_prefix_
 
+extern const int my_custom_prefix_var_const_name;
+
+extern int my_custom_prefix_var_mut_name;
+
 void my_custom_prefix_function_name(const int x);
 
diff --git a/bindgen-tests/tests/parse_callbacks/mod.rs b/bindgen-tests/tests/parse_callbacks/mod.rs
index 6ade71c2..00967fe8 100644
--- a/bindgen-tests/tests/parse_callbacks/mod.rs
+++ b/bindgen-tests/tests/parse_callbacks/mod.rs
@@ -1,22 +1,29 @@
 use bindgen::callbacks::*;
 
 #[derive(Debug)]
-pub struct RemoveFunctionPrefixParseCallback {
-    pub remove_function_prefix: Option<String>,
+pub struct RemovePrefixParseCallback {
+    pub remove_prefix: Option<String>,
 }
 
-impl RemoveFunctionPrefixParseCallback {
+impl RemovePrefixParseCallback {
     pub fn new(prefix: &str) -> Self {
-        RemoveFunctionPrefixParseCallback {
-            remove_function_prefix: Some(prefix.to_string()),
+        RemovePrefixParseCallback {
+            remove_prefix: Some(prefix.to_string()),
         }
     }
 }
 
-impl ParseCallbacks for RemoveFunctionPrefixParseCallback {
-    fn generated_name_override(&self, function_name: &str) -> Option<String> {
-        if let Some(prefix) = &self.remove_function_prefix {
-            if let Some(name) = function_name.strip_prefix(prefix) {
+impl ParseCallbacks for RemovePrefixParseCallback {
+    fn generated_name_override(&self, item_info: ItemInfo) -> Option<String> {
+        if let Some(prefix) = &self.remove_prefix {
+            let (expected_prefix, expected_suffix) = match item_info.kind {
+                ItemKind::Function => ("function_", "_name"),
+                ItemKind::Var => ("var_", "_name"),
+                _ => todo!(),
+            };
+            if let Some(name) = item_info.name.strip_prefix(prefix) {
+                assert!(name.starts_with(expected_prefix));
+                assert!(name.ends_with(expected_suffix));
                 return Some(name.to_string());
             }
         }
@@ -67,8 +74,7 @@ pub fn lookup(cb: &str) -> Box<dyn ParseCallbacks> {
                     .split("remove-function-prefix-")
                     .last()
                     .to_owned();
-                let lnopc =
-                    RemoveFunctionPrefixParseCallback::new(prefix.unwrap());
+                let lnopc = RemovePrefixParseCallback::new(prefix.unwrap());
                 Box::new(lnopc)
             } else {
                 panic!("Couldn't find name ParseCallbacks: {}", cb)