Skip to content

Commit

Permalink
Denote types with unused template params.
Browse files Browse the repository at this point in the history
Downstream postprocessors such as autocxx may want to use bindgen's
representation of types to generate additional C++ code. bindgen is remarkably
faithful at passing through enough information to make this possible - but for
some C++ types, bindgen chooses to elide some template parameters. It's not
safe for additional C++ code to be generated which references that type.

This adds a callback by which such tools can recognize types where template
parameters have been elided like this, so that extra C++ is avoided.

This information could be provided in the existing
`new_item_found` callback, but it's very niche and unlikely to be used by
the majority of consumers, so a new callback is added instead.

An alternative approach here is to provide a mode to bindgen where it
*always* uses all template params, by adding additional PhantomData
fields to structs where those params are not currently used.
This is being prototyped in google/autocxx#1425
but is unlikely to be successful, on the assumption that lots of the
templated types can't actually be properly represented by bindgen/Rust,
so the current strategy of discarding them is more likely to work
in the broad strokes.

Part of google/autocxx#124
  • Loading branch information
adetaylor committed Feb 22, 2025
1 parent 11d76b0 commit e6c675e
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 0 deletions.
1 change: 1 addition & 0 deletions bindgen-tests/tests/parse_callbacks/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod item_discovery_callback;
mod unused_template_param;

use bindgen::callbacks::*;
use bindgen::FieldVisibilityKind;
Expand Down
29 changes: 29 additions & 0 deletions bindgen-tests/tests/parse_callbacks/unused_template_param.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@

class NoTemplateParams {
public:
int a;
};

template<class T>
class UsesTemplateParam {
public:
UsesTemplateParam(T);
T t;
};

template<class A>
class IgnoresTemplateParam {
public:
IgnoresTemplateParam();
int a;
};

typedef NoTemplateParams TypedefNoTemplateParam;
using TypedefNoTemplateParam2 = NoTemplateParams;
typedef UsesTemplateParam<int> TypedefUsesTemplateParam;
using TypedefUsesTemplateParam2 = UsesTemplateParam<int>;
template <class T>
using TypedefUsesTemplateParam3 = UsesTemplateParam<T>;
typedef IgnoresTemplateParam<int> TypedefIgnoresTemplateParam;
template <class T>
using TypedefIgnoresTemplateParam2 = IgnoresTemplateParam<T>;
79 changes: 79 additions & 0 deletions bindgen-tests/tests/parse_callbacks/unused_template_param.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
use std::{
cell::RefCell,
collections::{HashMap, HashSet},
rc::Rc,
};

use bindgen::{
callbacks::{DiscoveredItem, DiscoveredItemId, ParseCallbacks},
Builder,
};

#[derive(Debug)]
struct UnusedTemplateParamCallback(
Rc<RefCell<HashSet<DiscoveredItemId>>>,
Rc<RefCell<HashMap<DiscoveredItemId, String>>>,
);

impl ParseCallbacks for UnusedTemplateParamCallback {
fn denote_discards_template_param(&self, id: DiscoveredItemId) {
self.0.borrow_mut().insert(id);
}

fn new_item_found(
&self,
id: DiscoveredItemId,
item: DiscoveredItem,
_source_location: Option<&bindgen::callbacks::SourceLocation>,
_parent: Option<DiscoveredItemId>,
) {
match item {
DiscoveredItem::Struct {
final_name: name, ..
} |
DiscoveredItem::Alias {
alias_name: name, ..
} => {
self.1.borrow_mut().insert(id, name);
}
_ => {}
}
}
}

/// This test could be combined with the `item_discovery_callback`
/// test, but as this is a separate callback we'll keep the tests
/// separate too.
#[test]
fn test_unused_template_param() {
let discovered = Rc::new(RefCell::new(HashSet::new()));
let names = Rc::new(RefCell::new(HashMap::new()));
let header_path = concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/parse_callbacks/unused_template_param.hpp"
);
Builder::default()
.header(header_path)
.parse_callbacks(Box::new(UnusedTemplateParamCallback(
Rc::clone(&discovered),
Rc::clone(&names),
)))
.clang_arg("--std=c++11")
.generate()
.expect("TODO: panic message");

let reported_unused_template_parameter_item_names: HashSet<_> = discovered
.borrow()
.iter()
.map(|id| names.borrow().get(id).expect("Name not reported").clone())
.collect();

assert_eq!(
reported_unused_template_parameter_item_names,
HashSet::from([
"IgnoresTemplateParam".to_string(),
"TypedefIgnoresTemplateParam2".to_string()
]),
"Different items than expected reported unused template params"
);
}
11 changes: 11 additions & 0 deletions bindgen/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,17 @@ pub trait ParseCallbacks: fmt::Debug {
) {
}

/// Notes that this type does not use all the template params
/// which were present in C++. Sometimes those template parameters
/// are not useful in Rust because they don't actually impact the
/// data stored in the type, so bindgen drops them. But this also means
/// that the bindgen output does not contain full and comprehensive
/// output about the original nature of the C++ type, so higher level
/// code generators may wish to behave differently. For example,
/// it would not be OK to attempt to generate additional C++ code
/// based on this.
fn denote_discards_template_param(&self, _id: DiscoveredItemId) {}

// TODO add callback for ResolvedTypeRef
}

Expand Down
24 changes: 24 additions & 0 deletions bindgen/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ use crate::ir::var::Var;

use proc_macro2::{Ident, Span};
use quote::{ToTokens, TokenStreamExt};
use utils::report_any_unused_template_params;

use crate::{Entry, HashMap, HashSet};
use std::borrow::Cow;
Expand Down Expand Up @@ -1022,6 +1023,13 @@ impl CodeGenerator for Type {
} else {
quote! {}
};
// We are handling (essentially) type X = Y;
// We want to denote only if X has unused template params, e.g.
// type X<A> = Y;
// We don't want to note whether Y has unused template params, e.g.
// type X = Y<B>
// because that information will be recorded against the definition of Y.
report_any_unused_template_params(ctx, item);

let alias_style = if ctx.options().type_alias.matches(&name) {
AliasVariation::TypeAlias
Expand Down Expand Up @@ -2444,6 +2452,7 @@ impl CodeGenerator for CompInfo {
} else {
attributes.push(attributes::repr("C"));
}
report_any_unused_template_params(ctx, item);

if true {
if let Some(explicit) = explicit_align {
Expand Down Expand Up @@ -5231,6 +5240,7 @@ pub(crate) mod utils {
use crate::ir::function::{Abi, ClangAbi, FunctionSig};
use crate::ir::item::{Item, ItemCanonicalPath};
use crate::ir::item_kind::ItemKind;
use crate::ir::template::TemplateParameters;
use crate::ir::ty::TypeKind;
use crate::{args_are_cpp, file_is_cpp};
use std::borrow::Cow;
Expand Down Expand Up @@ -6032,4 +6042,18 @@ pub(crate) mod utils {
_ => false,
}
}

/// Call the unused template param callback if needed.
pub(super) fn report_any_unused_template_params(
ctx: &BindgenContext,
item: &Item,
) {
ctx.options().for_each_callback(|cb| {
if item.has_unused_template_params(ctx) {
cb.denote_discards_template_param(DiscoveredItemId::new(
item.id().as_usize(),
));
}
})
}
}
17 changes: 17 additions & 0 deletions bindgen/ir/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,23 @@ pub(crate) trait TemplateParameters: Sized {
.filter(|p| ctx.uses_template_parameter(id, *p))
.collect()
}

/// Returns whether all the template parameters are used.
fn has_unused_template_params(&self, ctx: &BindgenContext) -> bool
where
Self: AsRef<ItemId>,
{
assert!(
ctx.in_codegen_phase(),
"template parameter usage is not computed until codegen"
);

let id = *self.as_ref();
ctx.resolve_item(id)
.all_template_params(ctx)
.into_iter()
.any(|p| !ctx.uses_template_parameter(id, p))
}
}

/// A trait for things which may or may not be a named template type parameter.
Expand Down

0 comments on commit e6c675e

Please sign in to comment.