From 4c9a01839ce04c1ecc03b740379ff5a70f124a7f Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Wed, 19 Feb 2025 15:32:29 +0000 Subject: [PATCH 1/7] Discovery callbacks for functions and methods. This extends the existing discovery callback mechanism to report on functions and methods. At this stage, we don't say much about them, in order to be consistent with other discovery callbacks. Subsequent PRs will add extra callbacks to provide information especially about methods (virtualness, C++ visibility, etc.) Please request changes if you think that sort of information should arrive in these callbacks. Because methods are a fundamentally C++ thing, this splits the current ParseCallbacks test to cover both a .h and a .hpp header. Part of https://github.com/google/autocxx/issues/124 --- .../header_item_discovery.h | 6 +- .../header_item_discovery.hpp | 6 + .../item_discovery_callback/mod.rs | 109 ++++++++++++++++-- bindgen/callbacks.rs | 17 ++- bindgen/codegen/mod.rs | 22 +++- 5 files changed, 150 insertions(+), 10 deletions(-) create mode 100644 bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h index b2bb04f15f..eb44e5fc58 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h @@ -25,4 +25,8 @@ enum NamedEnum { Fish, }; -typedef enum NamedEnum AliasOfNamedEnum; \ No newline at end of file +typedef enum NamedEnum AliasOfNamedEnum; + +// Functions + +void named_function(); diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp new file mode 100644 index 0000000000..1de8d99e4d --- /dev/null +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp @@ -0,0 +1,6 @@ +// Methods + +class SomeClass { +public: + void named_method(); +}; \ No newline at end of file diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs index 93a2b029d7..dc16aa2078 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs @@ -17,20 +17,26 @@ impl ParseCallbacks for ItemDiscovery { self.0.borrow_mut().insert(_id, _item); } } -#[test] -pub fn test_item_discovery_callback() { + +fn test_item_discovery_callback(header: &str, expected: HashMap) { let discovery = ItemDiscovery::default(); let info = Rc::clone(&discovery.0); + let mut header_path = env!("CARGO_MANIFEST_DIR").to_string(); + header_path.push_str(header); + Builder::default() - .header(concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h" - )) + .header(header_path) .parse_callbacks(Box::new(discovery)) .generate() .expect("TODO: panic message"); + + compare_item_caches(&info.borrow(), &expected); +} + +#[test] +fn test_item_discovery_callback_c() { let expected = ItemCache::from([ ( DiscoveredItemId::new(10), @@ -87,9 +93,38 @@ pub fn test_item_discovery_callback() { final_name: "_bindgen_ty_*".to_string(), }, ), + ( + DiscoveredItemId::new(41), + DiscoveredItem::Function { + final_name: "named_function".to_string(), + }, + ), ]); + test_item_discovery_callback( + "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h", expected); +} - compare_item_caches(&info.borrow(), &expected); + +#[test] +fn test_item_discovery_callback_cpp() { + let expected = ItemCache::from([ + ( + DiscoveredItemId::new(1), + DiscoveredItem::Struct { + original_name: Some("SomeClass".to_string()), + final_name: "SomeClass".to_string(), + }, + ), + ( + DiscoveredItemId::new(2), + DiscoveredItem::Method { + final_name: "named_method".to_string(), + parent: DiscoveredItemId::new(1), + }, + ), + ]); + test_item_discovery_callback( + "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp", expected); } pub fn compare_item_caches(generated: &ItemCache, expected: &ItemCache) { @@ -142,6 +177,12 @@ fn compare_item_info( DiscoveredItem::Enum { .. } => { compare_enum_info(expected_item, generated_item) } + DiscoveredItem::Function { .. } => { + compare_function_info(expected_item, generated_item) + } + DiscoveredItem::Method { .. } => { + compare_method_info(expected_item, generated_item) + } } } @@ -279,3 +320,57 @@ pub fn compare_alias_info( compare_item_info(expected_aliased, generated_aliased, expected, generated) } + +pub fn compare_function_info( + expected_item: &DiscoveredItem, + generated_item: &DiscoveredItem, +) -> bool { + let DiscoveredItem::Function { + final_name: expected_final_name, + } = expected_item + else { + unreachable!() + }; + + let DiscoveredItem::Function { + final_name: generated_final_name, + } = generated_item + else { + unreachable!() + }; + + if !compare_names(expected_final_name, generated_final_name) { + return false; + } + true +} + +pub fn compare_method_info( + expected_item: &DiscoveredItem, + generated_item: &DiscoveredItem, +) -> bool { + let DiscoveredItem::Method { + final_name: expected_final_name, + parent: expected_parent, + } = expected_item + else { + unreachable!() + }; + + let DiscoveredItem::Method { + final_name: generated_final_name, + parent: generated_parent, + } = generated_item + else { + unreachable!() + }; + + if expected_parent != generated_parent { + return false; + } + + if !compare_names(expected_final_name, generated_final_name) { + return false; + } + true +} diff --git a/bindgen/callbacks.rs b/bindgen/callbacks.rs index c2be66828a..579d7083aa 100644 --- a/bindgen/callbacks.rs +++ b/bindgen/callbacks.rs @@ -224,7 +224,22 @@ pub enum DiscoveredItem { /// The final name of the generated binding final_name: String, }, - // functions, modules, etc. + + /// A function or method. + Function { + /// The final name used. + final_name: String, + }, + + /// A method. + Method { + /// The final name used. + final_name: String, + + /// Type to which this method belongs. + parent: DiscoveredItemId, + } + // modules, etc. } /// Relevant information about a type to which new derive attributes will be added using diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index f5518e432d..c00d9d4300 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -2481,6 +2481,7 @@ impl CodeGenerator for CompInfo { let is_rust_union = is_union && struct_layout.is_rust_union(); + let discovered_id = DiscoveredItemId::new(item.id().as_usize()); ctx.options().for_each_callback(|cb| { let discovered_item = match self.kind() { CompKind::Struct => DiscoveredItem::Struct { @@ -2502,7 +2503,7 @@ impl CodeGenerator for CompInfo { }; cb.new_item_found( - DiscoveredItemId::new(item.id().as_usize()), + discovered_id, discovered_item, ); }); @@ -2711,6 +2712,7 @@ impl CodeGenerator for CompInfo { &mut method_names, result, self, + discovered_id, ); } } @@ -2729,6 +2731,7 @@ impl CodeGenerator for CompInfo { &mut method_names, result, self, + discovered_id, ); } } @@ -2742,6 +2745,7 @@ impl CodeGenerator for CompInfo { &mut method_names, result, self, + discovered_id, ); } } @@ -2999,6 +3003,7 @@ impl Method { method_names: &mut HashSet, result: &mut CodegenResult<'_>, _parent: &CompInfo, + parent_id: DiscoveredItemId, ) { assert!({ let cc = &ctx.options().codegen_config; @@ -3019,6 +3024,7 @@ impl Method { // First of all, output the actual function. let function_item = ctx.resolve_item(self.signature()); + let id = DiscoveredItemId::new(function_item.id().as_usize()); if !function_item.process_before_codegen(ctx, result) { return; } @@ -3065,6 +3071,11 @@ impl Method { method_names.insert(name.clone()); + ctx.options().for_each_callback(|cb| cb.new_item_found(id, DiscoveredItem::Method { + parent: parent_id, + final_name: name.clone(), + })); + let mut function_name = function_item.canonical_name(ctx); if times_seen > 0 { write!(&mut function_name, "{times_seen}").unwrap(); @@ -4540,6 +4551,7 @@ impl CodeGenerator for Function { ) -> Self::Return { debug!("::codegen: item = {item:?}"); debug_assert!(item.is_enabled_for_codegen(ctx)); + let id = DiscoveredItemId::new(item.id().as_usize()); let is_internal = matches!(self.linkage(), Linkage::Internal); @@ -4650,6 +4662,14 @@ impl CodeGenerator for Function { if times_seen > 0 { write!(&mut canonical_name, "{times_seen}").unwrap(); } + ctx.options().for_each_callback(|cb| { + cb.new_item_found( + id, + DiscoveredItem::Function { + final_name: canonical_name.to_string(), + } + ); + }); let link_name_attr = self.link_name().or_else(|| { let mangled_name = mangled_name.unwrap_or(name); From 67b12a779250617a36f2f221f91237a159302136 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Wed, 19 Feb 2025 16:00:06 +0000 Subject: [PATCH 2/7] Formatting tweaks. --- .../item_discovery_callback/mod.rs | 7 ++++--- bindgen/callbacks.rs | 3 +-- bindgen/codegen/mod.rs | 20 ++++++++++--------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs index dc16aa2078..645235b254 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs @@ -18,7 +18,10 @@ impl ParseCallbacks for ItemDiscovery { } } -fn test_item_discovery_callback(header: &str, expected: HashMap) { +fn test_item_discovery_callback( + header: &str, + expected: HashMap, +) { let discovery = ItemDiscovery::default(); let info = Rc::clone(&discovery.0); @@ -31,7 +34,6 @@ fn test_item_discovery_callback(header: &str, expected: HashMap 0 { @@ -4667,7 +4669,7 @@ impl CodeGenerator for Function { id, DiscoveredItem::Function { final_name: canonical_name.to_string(), - } + }, ); }); From cdaa002e7387d833f4430254e07fda313e81d875 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Thu, 20 Feb 2025 08:19:03 +0000 Subject: [PATCH 3/7] Deduplicate logic to call discovery callback. No functional change - just deduplicating the logic which calls this callback, which will make it easier to make further changes in future. Part of https://github.com/google/autocxx/issues/124 --- bindgen/codegen/mod.rs | 108 ++++++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 56 deletions(-) diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 903af6df6c..d9ced58995 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -995,16 +995,13 @@ impl CodeGenerator for Type { let rust_name = ctx.rust_ident(&name); - ctx.options().for_each_callback(|cb| { - cb.new_item_found( - DiscoveredItemId::new(item.id().as_usize()), - DiscoveredItem::Alias { - alias_name: rust_name.to_string(), - alias_for: DiscoveredItemId::new( - inner_item.id().as_usize(), - ), - }, - ); + utils::call_discovered_item_callback(ctx, item, || { + DiscoveredItem::Alias { + alias_name: rust_name.to_string(), + alias_for: DiscoveredItemId::new( + inner_item.id().as_usize(), + ), + } }); let mut tokens = if let Some(comment) = item.comment(ctx) { @@ -2481,28 +2478,23 @@ impl CodeGenerator for CompInfo { let is_rust_union = is_union && struct_layout.is_rust_union(); - let discovered_id = DiscoveredItemId::new(item.id().as_usize()); - ctx.options().for_each_callback(|cb| { - let discovered_item = match self.kind() { - CompKind::Struct => DiscoveredItem::Struct { - original_name: item - .kind() - .expect_type() - .name() - .map(String::from), - final_name: canonical_ident.to_string(), - }, - CompKind::Union => DiscoveredItem::Union { - original_name: item - .kind() - .expect_type() - .name() - .map(String::from), - final_name: canonical_ident.to_string(), - }, - }; - - cb.new_item_found(discovered_id, discovered_item); + utils::call_discovered_item_callback(ctx, item, || match self.kind() { + CompKind::Struct => DiscoveredItem::Struct { + original_name: item + .kind() + .expect_type() + .name() + .map(String::from), + final_name: canonical_ident.to_string(), + }, + CompKind::Union => DiscoveredItem::Union { + original_name: item + .kind() + .expect_type() + .name() + .map(String::from), + final_name: canonical_ident.to_string(), + }, }); // The custom derives callback may return a list of derive attributes; @@ -2700,6 +2692,7 @@ impl CodeGenerator for CompInfo { } let mut method_names = Default::default(); + let discovered_id = DiscoveredItemId::new(item.id().as_usize()); if ctx.options().codegen_config.methods() { for method in self.methods() { assert_ne!(method.kind(), MethodKind::Constructor); @@ -3021,7 +3014,6 @@ impl Method { // First of all, output the actual function. let function_item = ctx.resolve_item(self.signature()); - let id = DiscoveredItemId::new(function_item.id().as_usize()); if !function_item.process_before_codegen(ctx, result) { return; } @@ -3068,14 +3060,11 @@ impl Method { method_names.insert(name.clone()); - ctx.options().for_each_callback(|cb| { - cb.new_item_found( - id, - DiscoveredItem::Method { - parent: parent_id, - final_name: name.clone(), - }, - ) + utils::call_discovered_item_callback(ctx, function_item, || { + DiscoveredItem::Method { + parent: parent_id, + final_name: name.clone(), + } }); let mut function_name = function_item.canonical_name(ctx); @@ -3783,13 +3772,10 @@ impl CodeGenerator for Enum { let repr = repr.to_rust_ty_or_opaque(ctx, item); let has_typedef = ctx.is_enum_typedef_combo(item.id()); - ctx.options().for_each_callback(|cb| { - cb.new_item_found( - DiscoveredItemId::new(item.id().as_usize()), - DiscoveredItem::Enum { - final_name: name.to_string(), - }, - ); + utils::call_discovered_item_callback(ctx, item, || { + DiscoveredItem::Enum { + final_name: name.to_string(), + } }); let mut builder = @@ -4553,7 +4539,6 @@ impl CodeGenerator for Function { ) -> Self::Return { debug!("::codegen: item = {item:?}"); debug_assert!(item.is_enabled_for_codegen(ctx)); - let id = DiscoveredItemId::new(item.id().as_usize()); let is_internal = matches!(self.linkage(), Linkage::Internal); @@ -4664,13 +4649,10 @@ impl CodeGenerator for Function { if times_seen > 0 { write!(&mut canonical_name, "{times_seen}").unwrap(); } - ctx.options().for_each_callback(|cb| { - cb.new_item_found( - id, - DiscoveredItem::Function { - final_name: canonical_name.to_string(), - }, - ); + utils::call_discovered_item_callback(ctx, item, || { + DiscoveredItem::Function { + final_name: canonical_name.to_string(), + } }); let link_name_attr = self.link_name().or_else(|| { @@ -5211,6 +5193,7 @@ pub(crate) mod utils { use super::helpers::BITFIELD_UNIT; use super::serialize::CSerialize; use super::{error, CodegenError, CodegenResult, ToRustTyOrOpaque}; + use crate::callbacks::DiscoveredItemId; use crate::ir::context::BindgenContext; use crate::ir::context::TypeId; use crate::ir::function::{Abi, ClangAbi, FunctionSig}; @@ -5940,4 +5923,17 @@ pub(crate) mod utils { true } + + pub(super) fn call_discovered_item_callback( + ctx: &BindgenContext, + item: &Item, + discovered_item_creator: impl Fn() -> crate::callbacks::DiscoveredItem, + ) { + ctx.options().for_each_callback(|cb| { + cb.new_item_found( + DiscoveredItemId::new(item.id().as_usize()), + discovered_item_creator(), + ); + }); + } } From f607994c3ab87ee754ce7a9138564c2a22b6bb92 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Wed, 19 Feb 2025 16:54:05 +0000 Subject: [PATCH 4/7] Include source location in callback. This adds more information to ParseCallbacks which indicates the location in the original source code at which a given item was found. This has proven to be useful in downstream code generators in providing diagnostics to explain why a given item can't be represented in Rust. (There are lots of reasons why this might not be the case - autocxx has around 100 which can be found here - https://github.com/google/autocxx/blob/d85eac76c9b3089d0d86249e857ff0e8c36b988f/engine/src/conversion/convert_error.rs#L39 - but irrespective of the specific reasons, it's useful to be able to point to the original location when emitting diagnostics). Should we make this a new callback or include this information within the existing callback? Pros of making it a new callback: * No compatibility breakage. Pros of including it in this existing callback: * No need to specify and test a policy about whether such callbacks always happen together, or may arrive individually * Easier for recipients (including bindgen's own test suite) to keep track of the source code location received. * Because we add new items to the DiscoveryItem enum anyway, we seem to have accepted it's OK to break compatibility in this callback (for now at least). Therefore I'm adding it as a parameter to the existing callback. If it's deemed acceptable to break compatibility in this way, I will follow the same thought process for some other changes too. Part of https://github.com/google/autocxx/issues/124. --- .../item_discovery_callback/mod.rs | 284 +++++++++++++----- bindgen/callbacks.rs | 16 +- bindgen/codegen/mod.rs | 11 + 3 files changed, 236 insertions(+), 75 deletions(-) diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs index 645235b254..36852e6465 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs @@ -4,23 +4,57 @@ use std::rc::Rc; use regex::Regex; -use bindgen::callbacks::{DiscoveredItem, DiscoveredItemId, ParseCallbacks}; +use bindgen::callbacks::{ + DiscoveredItem, DiscoveredItemId, ParseCallbacks, SourceLocation, +}; use bindgen::Builder; #[derive(Debug, Default)] struct ItemDiscovery(Rc>); -pub type ItemCache = HashMap; +pub type ItemCache = HashMap; + +#[derive(Debug)] +pub struct DiscoveredInformation(DiscoveredItem, Option); impl ParseCallbacks for ItemDiscovery { - fn new_item_found(&self, _id: DiscoveredItemId, _item: DiscoveredItem) { - self.0.borrow_mut().insert(_id, _item); + fn new_item_found( + &self, + id: DiscoveredItemId, + item: DiscoveredItem, + source_location: Option<&SourceLocation>, + ) { + self.0 + .borrow_mut() + .insert(id, DiscoveredInformation(item, source_location.cloned())); } } +#[derive(Debug)] +pub struct ItemExpectations { + item: DiscoveredItem, + source_location: Option<(usize, usize, usize)>, +} + +impl ItemExpectations { + fn new( + item: DiscoveredItem, + line: usize, + col: usize, + byte_offset: usize, + ) -> Self { + Self { + item, + source_location: Some((line, col, byte_offset)), + } + } +} + +type ExpectationMap = HashMap; + fn test_item_discovery_callback( header: &str, - expected: HashMap, + expected: HashMap, ) { let discovery = ItemDiscovery::default(); let info = Rc::clone(&discovery.0); @@ -34,72 +68,117 @@ fn test_item_discovery_callback( .generate() .expect("TODO: panic message"); - compare_item_caches(&info.borrow(), &expected); + compare_item_caches(&info.borrow(), &expected, header); } #[test] fn test_item_discovery_callback_c() { - let expected = ItemCache::from([ + let expected = ExpectationMap::from([ ( DiscoveredItemId::new(10), - DiscoveredItem::Struct { - original_name: Some("NamedStruct".to_string()), - final_name: "NamedStruct".to_string(), - }, + ItemExpectations::new( + DiscoveredItem::Struct { + original_name: Some("NamedStruct".to_string()), + final_name: "NamedStruct".to_string(), + }, + 4, + 8, + 73, + ), ), ( DiscoveredItemId::new(11), - DiscoveredItem::Alias { - alias_name: "AliasOfNamedStruct".to_string(), - alias_for: DiscoveredItemId::new(10), - }, + ItemExpectations::new( + DiscoveredItem::Alias { + alias_name: "AliasOfNamedStruct".to_string(), + alias_for: DiscoveredItemId::new(10), + }, + 7, + 28, + 118, + ), ), ( DiscoveredItemId::new(20), - DiscoveredItem::Union { - original_name: Some("NamedUnion".to_string()), - final_name: "NamedUnion".to_string(), - }, + ItemExpectations::new( + DiscoveredItem::Union { + original_name: Some("NamedUnion".to_string()), + final_name: "NamedUnion".to_string(), + }, + 13, + 7, + 209, + ), ), ( DiscoveredItemId::new(21), - DiscoveredItem::Alias { - alias_name: "AliasOfNamedUnion".to_string(), - alias_for: DiscoveredItemId::new(20), - }, + ItemExpectations::new( + DiscoveredItem::Alias { + alias_name: "AliasOfNamedUnion".to_string(), + alias_for: DiscoveredItemId::new(20), + }, + 16, + 26, + 251, + ), ), ( DiscoveredItemId::new(27), - DiscoveredItem::Alias { - alias_name: "AliasOfNamedEnum".to_string(), - alias_for: DiscoveredItemId::new(24), - }, + ItemExpectations::new( + DiscoveredItem::Alias { + alias_name: "AliasOfNamedEnum".to_string(), + alias_for: DiscoveredItemId::new(24), + }, + 28, + 24, + 515, + ), ), ( DiscoveredItemId::new(24), - DiscoveredItem::Enum { - final_name: "NamedEnum".to_string(), - }, + ItemExpectations::new( + DiscoveredItem::Enum { + final_name: "NamedEnum".to_string(), + }, + 24, + 6, + 466, + ), ), ( DiscoveredItemId::new(30), - DiscoveredItem::Struct { - original_name: None, - final_name: "_bindgen_ty_*".to_string(), - }, + ItemExpectations::new( + DiscoveredItem::Struct { + original_name: None, + final_name: "_bindgen_ty_*".to_string(), + }, + 2, + 38, + 48, + ), ), ( DiscoveredItemId::new(40), - DiscoveredItem::Union { - original_name: None, - final_name: "_bindgen_ty_*".to_string(), - }, + ItemExpectations::new( + DiscoveredItem::Union { + original_name: None, + final_name: "_bindgen_ty_*".to_string(), + }, + 11, + 37, + 186, + ), ), ( DiscoveredItemId::new(41), - DiscoveredItem::Function { - final_name: "named_function".to_string(), - }, + ItemExpectations::new( + DiscoveredItem::Function { + final_name: "named_function".to_string(), + }, + 32, + 6, + 553, + ), ), ]); test_item_discovery_callback( @@ -108,40 +187,58 @@ fn test_item_discovery_callback_c() { #[test] fn test_item_discovery_callback_cpp() { - let expected = ItemCache::from([ + let expected = ExpectationMap::from([ ( DiscoveredItemId::new(1), - DiscoveredItem::Struct { - original_name: Some("SomeClass".to_string()), - final_name: "SomeClass".to_string(), - }, + ItemExpectations::new( + DiscoveredItem::Struct { + original_name: Some("SomeClass".to_string()), + final_name: "SomeClass".to_string(), + }, + 3, + 7, + 18, + ), ), ( DiscoveredItemId::new(2), - DiscoveredItem::Method { - final_name: "named_method".to_string(), - parent: DiscoveredItemId::new(1), - }, + ItemExpectations::new( + DiscoveredItem::Method { + final_name: "named_method".to_string(), + parent: DiscoveredItemId::new(1), + }, + 5, + 10, + 47, + ), ), ]); test_item_discovery_callback( "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp", expected); } -pub fn compare_item_caches(generated: &ItemCache, expected: &ItemCache) { +fn compare_item_caches( + generated: &ItemCache, + expected: &ExpectationMap, + expected_filename: &str, +) { // We can't use a simple Eq::eq comparison because of two reasons: // - anonymous structs/unions will have a final name generated by bindgen which may change // if the header file or the bindgen logic is altered // - aliases have a DiscoveredItemId that we can't directly compare for the same instability reasons for expected_item in expected.values() { - let found = generated.iter().find(|(_generated_id, generated_item)| { - compare_item_info( - expected_item, - generated_item, - expected, - generated, - ) - }); + let found = + generated + .iter() + .find(|(_generated_id, generated_item)| { + compare_item_info( + expected_item, + generated_item, + expected, + generated, + expected_filename, + ) + }); assert!( found.is_some(), @@ -151,40 +248,72 @@ pub fn compare_item_caches(generated: &ItemCache, expected: &ItemCache) { } fn compare_item_info( - expected_item: &DiscoveredItem, - generated_item: &DiscoveredItem, - expected: &ItemCache, + expected_item: &ItemExpectations, + generated_item: &DiscoveredInformation, + expected: &ExpectationMap, generated: &ItemCache, + expected_filename: &str, ) -> bool { - if std::mem::discriminant(expected_item) != - std::mem::discriminant(generated_item) + if std::mem::discriminant(&expected_item.item) + != std::mem::discriminant(&generated_item.0) { return false; } - match generated_item { + let is_a_match = match generated_item.0 { DiscoveredItem::Struct { .. } => { - compare_struct_info(expected_item, generated_item) + compare_struct_info(&expected_item.item, &generated_item.0) } DiscoveredItem::Union { .. } => { - compare_union_info(expected_item, generated_item) + compare_union_info(&expected_item.item, &generated_item.0) } DiscoveredItem::Alias { .. } => compare_alias_info( - expected_item, - generated_item, + &expected_item.item, + &generated_item.0, expected, generated, + expected_filename, ), DiscoveredItem::Enum { .. } => { - compare_enum_info(expected_item, generated_item) + compare_enum_info(&expected_item.item, &generated_item.0) } DiscoveredItem::Function { .. } => { - compare_function_info(expected_item, generated_item) + compare_function_info(&expected_item.item, &generated_item.0) } DiscoveredItem::Method { .. } => { - compare_method_info(expected_item, generated_item) + compare_method_info(&expected_item.item, &generated_item.0) + } + }; + + if is_a_match { + // Compare source location + assert!( + generated_item.1.is_some() + == expected_item.source_location.is_some(), + "No source location provided when one was expected" + ); + if let Some(generated_location) = generated_item.1.as_ref() { + let expected_location = expected_item.source_location.unwrap(); + assert!( + generated_location + .file_name + .as_ref() + .expect("No filename provided") + .ends_with(expected_filename), + "Filename differed" + ); + assert_eq!( + ( + generated_location.line, + generated_location.col, + generated_location.byte_offset + ), + expected_location, + "Line/col/offsets differ" + ); } } + is_a_match } pub fn compare_names(expected_name: &str, generated_name: &str) -> bool { @@ -288,8 +417,9 @@ pub fn compare_enum_info( pub fn compare_alias_info( expected_item: &DiscoveredItem, generated_item: &DiscoveredItem, - expected: &ItemCache, + expected: &ExpectationMap, generated: &ItemCache, + expected_filename: &str, ) -> bool { let DiscoveredItem::Alias { alias_name: expected_alias_name, @@ -319,7 +449,13 @@ pub fn compare_alias_info( return false; }; - compare_item_info(expected_aliased, generated_aliased, expected, generated) + compare_item_info( + expected_aliased, + generated_aliased, + expected, + generated, + expected_filename, + ) } pub fn compare_function_info( diff --git a/bindgen/callbacks.rs b/bindgen/callbacks.rs index cd7bb9773d..c67fb519b3 100644 --- a/bindgen/callbacks.rs +++ b/bindgen/callbacks.rs @@ -164,7 +164,7 @@ pub trait ParseCallbacks: fmt::Debug { } /// This will get called everytime an item (currently struct, union, and alias) is found with some information about it - fn new_item_found(&self, _id: DiscoveredItemId, _item: DiscoveredItem) {} + fn new_item_found(&self, _id: DiscoveredItemId, _item: DiscoveredItem, _source_location: Option<&SourceLocation>) {} // TODO add callback for ResolvedTypeRef } @@ -304,3 +304,17 @@ pub struct FieldInfo<'a> { /// The name of the type of the field. pub field_type_name: Option<&'a str>, } + +/// Location in the source code. Roughly equivalent to the same type +/// within `clang_sys`. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SourceLocation { + /// Line number. + pub line: usize, + /// Column number within line. + pub col: usize, + /// Byte offset within file. + pub byte_offset: usize, + /// Filename, if known. + pub file_name: Option, +} diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index d9ced58995..322922b463 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -5929,10 +5929,21 @@ pub(crate) mod utils { item: &Item, discovered_item_creator: impl Fn() -> crate::callbacks::DiscoveredItem, ) { + let source_location = item.location().map(|clang_location| { + let (file, line, col, byte_offset) = clang_location.location(); + let file_name = file.name(); + crate::callbacks::SourceLocation { + line, + col, + byte_offset, + file_name, + } + }); ctx.options().for_each_callback(|cb| { cb.new_item_found( DiscoveredItemId::new(item.id().as_usize()), discovered_item_creator(), + source_location.as_ref(), ); }); } From 743c4698d7015b68b90c9b80aecafc5c65cd2444 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Thu, 20 Feb 2025 13:09:01 +0000 Subject: [PATCH 5/7] Clippy formatting fixes. --- .../item_discovery_callback/mod.rs | 29 +++++++++---------- bindgen/callbacks.rs | 8 ++++- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs index 36852e6465..6f64bd2f10 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs @@ -227,18 +227,15 @@ fn compare_item_caches( // if the header file or the bindgen logic is altered // - aliases have a DiscoveredItemId that we can't directly compare for the same instability reasons for expected_item in expected.values() { - let found = - generated - .iter() - .find(|(_generated_id, generated_item)| { - compare_item_info( - expected_item, - generated_item, - expected, - generated, - expected_filename, - ) - }); + let found = generated.iter().find(|(_generated_id, generated_item)| { + compare_item_info( + expected_item, + generated_item, + expected, + generated, + expected_filename, + ) + }); assert!( found.is_some(), @@ -254,8 +251,8 @@ fn compare_item_info( generated: &ItemCache, expected_filename: &str, ) -> bool { - if std::mem::discriminant(&expected_item.item) - != std::mem::discriminant(&generated_item.0) + if std::mem::discriminant(&expected_item.item) != + std::mem::discriminant(&generated_item.0) { return false; } @@ -288,8 +285,8 @@ fn compare_item_info( if is_a_match { // Compare source location assert!( - generated_item.1.is_some() - == expected_item.source_location.is_some(), + generated_item.1.is_some() == + expected_item.source_location.is_some(), "No source location provided when one was expected" ); if let Some(generated_location) = generated_item.1.as_ref() { diff --git a/bindgen/callbacks.rs b/bindgen/callbacks.rs index c67fb519b3..329cb888c1 100644 --- a/bindgen/callbacks.rs +++ b/bindgen/callbacks.rs @@ -164,7 +164,13 @@ pub trait ParseCallbacks: fmt::Debug { } /// This will get called everytime an item (currently struct, union, and alias) is found with some information about it - fn new_item_found(&self, _id: DiscoveredItemId, _item: DiscoveredItem, _source_location: Option<&SourceLocation>) {} + fn new_item_found( + &self, + _id: DiscoveredItemId, + _item: DiscoveredItem, + _source_location: Option<&SourceLocation>, + ) { + } // TODO add callback for ResolvedTypeRef } From 8dbc5b695f23d2222d8358e963075afd338dc215 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Fri, 21 Feb 2025 09:56:53 +0000 Subject: [PATCH 6/7] Report mods in callbacks, and item parentage. This makes two complementary improvements to the ParseCallbacks. The first is that Mods are now announced, as a new type of DiscoveredItem. The second is that the parentage of each item is announced. The parent of an item is often a mod (i.e. a C++ namespace) but not necessarily - it might be a struct within a struct, or similar. The reported information here is dependent on two pre-existing bindgen options: * whether to report C++ namespaces at all * whether to report inline namespaces conservatively. For that reason, the test suite gains two new tests. Part of https://github.com/google/autocxx/issues/124 --- .../header_item_discovery_with_namespaces.hpp | 30 ++ .../item_discovery_callback/mod.rs | 336 +++++++++++++++++- bindgen/callbacks.rs | 29 +- bindgen/codegen/mod.rs | 66 ++++ 4 files changed, 445 insertions(+), 16 deletions(-) create mode 100644 bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery_with_namespaces.hpp diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery_with_namespaces.hpp b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery_with_namespaces.hpp new file mode 100644 index 0000000000..ad8d4a524f --- /dev/null +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/header_item_discovery_with_namespaces.hpp @@ -0,0 +1,30 @@ +void a(); + +namespace B { + void c(); + + namespace D { + void e(); + } + + // We should not report empty namespaces + namespace F { + } + + namespace { + void g(); + } + + inline namespace H { + void i(); + namespace J { + void k(); + } + } + + struct L { + struct M { + + }; + }; +}; \ No newline at end of file diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs index 6f64bd2f10..2f64b502dd 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs @@ -1,5 +1,6 @@ use std::cell::RefCell; use std::collections::HashMap; +use std::convert::identity; use std::rc::Rc; use regex::Regex; @@ -15,7 +16,11 @@ struct ItemDiscovery(Rc>); pub type ItemCache = HashMap; #[derive(Debug)] -pub struct DiscoveredInformation(DiscoveredItem, Option); +pub struct DiscoveredInformation( + DiscoveredItem, + Option, + Option, +); impl ParseCallbacks for ItemDiscovery { fn new_item_found( @@ -23,10 +28,12 @@ impl ParseCallbacks for ItemDiscovery { id: DiscoveredItemId, item: DiscoveredItem, source_location: Option<&SourceLocation>, + parent: Option, ) { - self.0 - .borrow_mut() - .insert(id, DiscoveredInformation(item, source_location.cloned())); + self.0.borrow_mut().insert( + id, + DiscoveredInformation(item, source_location.cloned(), parent), + ); } } @@ -34,6 +41,7 @@ impl ParseCallbacks for ItemDiscovery { pub struct ItemExpectations { item: DiscoveredItem, source_location: Option<(usize, usize, usize)>, + parent: Option, } impl ItemExpectations { @@ -42,19 +50,33 @@ impl ItemExpectations { line: usize, col: usize, byte_offset: usize, + parent: Option, ) -> Self { Self { item, source_location: Some((line, col, byte_offset)), + parent, + } + } + + fn new_no_source_location( + item: DiscoveredItem, + parent: Option, + ) -> Self { + Self { + item, + source_location: None, + parent, } } } type ExpectationMap = HashMap; -fn test_item_discovery_callback( +fn test_item_discovery_callback Builder>( header: &str, expected: HashMap, + builder_adjuster: F, ) { let discovery = ItemDiscovery::default(); let info = Rc::clone(&discovery.0); @@ -62,11 +84,10 @@ fn test_item_discovery_callback( let mut header_path = env!("CARGO_MANIFEST_DIR").to_string(); header_path.push_str(header); - Builder::default() + let b = Builder::default() .header(header_path) - .parse_callbacks(Box::new(discovery)) - .generate() - .expect("TODO: panic message"); + .parse_callbacks(Box::new(discovery)); + builder_adjuster(b).generate().expect("TODO: panic message"); compare_item_caches(&info.borrow(), &expected, header); } @@ -84,6 +105,7 @@ fn test_item_discovery_callback_c() { 4, 8, 73, + None, ), ), ( @@ -96,6 +118,7 @@ fn test_item_discovery_callback_c() { 7, 28, 118, + None, ), ), ( @@ -108,6 +131,7 @@ fn test_item_discovery_callback_c() { 13, 7, 209, + None, ), ), ( @@ -120,6 +144,7 @@ fn test_item_discovery_callback_c() { 16, 26, 251, + None, ), ), ( @@ -132,6 +157,7 @@ fn test_item_discovery_callback_c() { 28, 24, 515, + None, ), ), ( @@ -143,6 +169,7 @@ fn test_item_discovery_callback_c() { 24, 6, 466, + None, ), ), ( @@ -155,6 +182,7 @@ fn test_item_discovery_callback_c() { 2, 38, 48, + None, ), ), ( @@ -167,6 +195,7 @@ fn test_item_discovery_callback_c() { 11, 37, 186, + None, ), ), ( @@ -178,11 +207,12 @@ fn test_item_discovery_callback_c() { 32, 6, 553, + None, ), ), ]); test_item_discovery_callback( - "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h", expected); + "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h", expected, identity); } #[test] @@ -198,6 +228,7 @@ fn test_item_discovery_callback_cpp() { 3, 7, 18, + None, ), ), ( @@ -210,11 +241,231 @@ fn test_item_discovery_callback_cpp() { 5, 10, 47, + None, ), ), ]); test_item_discovery_callback( - "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp", expected); + "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp", expected, identity); +} + +/// Returns the expectations corresponding to header_item_discovery_with_namespaces.hpp, +/// other than those items whose behavior changes based on the setting for +/// conservative inline namespaces, which we test each way. +fn cpp_expectation_map() -> ExpectationMap { + ExpectationMap::from([ + ( + DiscoveredItemId::new(0), + ItemExpectations::new_no_source_location( + DiscoveredItem::Mod { + final_name: "".to_string(), + anonymous: false, + inline: false, + }, + None, + ), + ), + ( + DiscoveredItemId::new(4), + ItemExpectations::new( + DiscoveredItem::Function { + final_name: "a".to_string(), + }, + 1, + 6, + 5, + Some(DiscoveredItemId::new(0)), + ), + ), + ( + DiscoveredItemId::new(5), + ItemExpectations::new( + DiscoveredItem::Mod { + final_name: "B".to_string(), + anonymous: false, + inline: false, + }, + 3, + 11, + 21, + Some(DiscoveredItemId::new(0)), + ), + ), + ( + DiscoveredItemId::new(9), + ItemExpectations::new( + DiscoveredItem::Function { + final_name: "c".to_string(), + }, + 4, + 10, + 34, + Some(DiscoveredItemId::new(5)), + ), + ), + ( + DiscoveredItemId::new(10), + ItemExpectations::new( + DiscoveredItem::Mod { + final_name: "D".to_string(), + anonymous: false, + inline: false, + }, + 6, + 15, + 54, + Some(DiscoveredItemId::new(5)), + ), + ), + ( + DiscoveredItemId::new(14), + ItemExpectations::new( + DiscoveredItem::Function { + final_name: "e".to_string(), + }, + 7, + 14, + 71, + Some(DiscoveredItemId::new(10)), + ), + ), + ( + DiscoveredItemId::new(16), + ItemExpectations::new( + DiscoveredItem::Mod { + final_name: "".to_string(), + anonymous: true, + inline: false, + }, + 14, + 15, + 167, + Some(DiscoveredItemId::new(5)), + ), + ), + ( + DiscoveredItemId::new(30), + ItemExpectations::new( + DiscoveredItem::Function { + final_name: "k".to_string(), + }, + 21, + 18, + 276, + Some(DiscoveredItemId::new(26)), + ), + ), + ( + DiscoveredItemId::new(31), + ItemExpectations::new( + DiscoveredItem::Struct { + final_name: "L".to_string(), + original_name: Some("L".to_string()), + }, + 25, + 12, + 309, + Some(DiscoveredItemId::new(5)), + ), + ), + ( + DiscoveredItemId::new(32), + ItemExpectations::new( + DiscoveredItem::Struct { + final_name: "L_M".to_string(), + original_name: Some("M".to_string()), + }, + 26, + 16, + 328, + Some(DiscoveredItemId::new(31)), + ), + ), + ]) +} + +#[test] +fn test_item_discovery_callback_cpp_namespaces_no_inline_namespaces() { + let mut expected = cpp_expectation_map(); + expected.insert( + DiscoveredItemId::new(25), + ItemExpectations::new( + DiscoveredItem::Function { + final_name: "i".to_string(), + }, + 19, + 14, + 232, + Some(DiscoveredItemId::new(5)), + ), + ); + expected.insert( + DiscoveredItemId::new(26), + ItemExpectations::new( + DiscoveredItem::Mod { + final_name: "J".to_string(), + anonymous: false, + inline: false, + }, + 20, + 19, + 255, + Some(DiscoveredItemId::new(5)), + ), + ); + + // C++11 for inline namespace + test_item_discovery_callback( + "/tests/parse_callbacks/item_discovery_callback/header_item_discovery_with_namespaces.hpp", expected, |b| b.enable_cxx_namespaces().clang_arg("--std=c++11")); +} + +#[test] +fn test_item_discovery_callback_cpp_namespaces_with_inline_namespaces() { + let mut expected = cpp_expectation_map(); + expected.insert( + DiscoveredItemId::new(21), + ItemExpectations::new( + DiscoveredItem::Mod { + final_name: "H".to_string(), + anonymous: false, + inline: true, + }, + 18, + 22, + 215, + Some(DiscoveredItemId::new(5)), + ), + ); + expected.insert( + DiscoveredItemId::new(25), + ItemExpectations::new( + DiscoveredItem::Function { + final_name: "i".to_string(), + }, + 19, + 14, + 232, + Some(DiscoveredItemId::new(21)), + ), + ); + expected.insert( + DiscoveredItemId::new(26), + ItemExpectations::new( + DiscoveredItem::Mod { + final_name: "J".to_string(), + anonymous: false, + inline: false, + }, + 20, + 19, + 255, + Some(DiscoveredItemId::new(21)), + ), + ); + + // C++11 for inline namespace + test_item_discovery_callback( + "/tests/parse_callbacks/item_discovery_callback/header_item_discovery_with_namespaces.hpp", expected, |b| b.enable_cxx_namespaces().conservative_inline_namespaces().clang_arg("--std=c++11")); } fn compare_item_caches( @@ -251,8 +502,8 @@ fn compare_item_info( generated: &ItemCache, expected_filename: &str, ) -> bool { - if std::mem::discriminant(&expected_item.item) != - std::mem::discriminant(&generated_item.0) + if std::mem::discriminant(&expected_item.item) + != std::mem::discriminant(&generated_item.0) { return false; } @@ -280,6 +531,9 @@ fn compare_item_info( DiscoveredItem::Method { .. } => { compare_method_info(&expected_item.item, &generated_item.0) } + DiscoveredItem::Mod { .. } => { + compare_mod_info(&expected_item.item, &generated_item.0) + } }; if is_a_match { @@ -287,7 +541,7 @@ fn compare_item_info( assert!( generated_item.1.is_some() == expected_item.source_location.is_some(), - "No source location provided when one was expected" + "Source location wasn't as expected for generated={generated_item:?}, expected={expected_item:?}" ); if let Some(generated_location) = generated_item.1.as_ref() { let expected_location = expected_item.source_location.unwrap(); @@ -309,6 +563,21 @@ fn compare_item_info( "Line/col/offsets differ" ); } + + // Compare C++ name info + assert!( + generated_item.2.is_some() == + expected_item.parent.is_some(), + "Parent information didn't match: generated item {generated_item:?}" + ); + + if let Some(generated_parent) = generated_item.2.as_ref() { + let expected_parent = expected_item.parent.as_ref().unwrap(); + assert_eq!( + generated_parent, expected_parent, + "Parent didn't match for {expected_item:?}" + ); + } } is_a_match } @@ -508,3 +777,42 @@ pub fn compare_method_info( } true } + +pub fn compare_mod_info( + expected_item: &DiscoveredItem, + generated_item: &DiscoveredItem, +) -> bool { + let DiscoveredItem::Mod { + final_name: expected_final_name, + anonymous: expected_anonymous, + inline: expected_inline, + } = expected_item + else { + unreachable!() + }; + + let DiscoveredItem::Mod { + final_name: generated_final_name, + anonymous: generated_anonymous, + inline: generated_inline, + } = generated_item + else { + unreachable!() + }; + + if expected_anonymous != generated_anonymous + || *expected_inline != *generated_inline + { + return false; + } + + // We generate arbitrary names for anonymous namespaces - do not compare + if !expected_anonymous { + // Do not use regexes to compare mod names since the root mod + // has an empty name and would match everything + if expected_final_name != generated_final_name { + return false; + } + } + true +} diff --git a/bindgen/callbacks.rs b/bindgen/callbacks.rs index 329cb888c1..5cbc1e2b17 100644 --- a/bindgen/callbacks.rs +++ b/bindgen/callbacks.rs @@ -163,12 +163,22 @@ pub trait ParseCallbacks: fmt::Debug { None } - /// This will get called everytime an item (currently struct, union, and alias) is found with some information about it + /// This will get called everytime an item is found with some information about it. + /// `_parent` is the location in which the item has been found, if any. + /// This is guaranteed to be a [`DiscoveredItem`] as reported + /// by [`ParseCallbacks::new_item_found`], most likely a + /// [`DiscoveredItem::Mod`] but perhaps something else such as a + /// [`DiscoveredItem::Struct`]. + /// If C++ namespace support has not been enabled in bindgen's options, + /// most items will have no declared `_parent`. If C++ namespace support + /// has been enabled, all items should have a parent other than the root + /// namespace. fn new_item_found( &self, _id: DiscoveredItemId, _item: DiscoveredItem, _source_location: Option<&SourceLocation>, + _parent: Option, ) { } @@ -231,6 +241,21 @@ pub enum DiscoveredItem { final_name: String, }, + /// A module, representing a C++ namespace. + /// The root module can be identified by the fact that it has a `None` + /// parent declared within [`ParseCallbacks::new_item_found`]. + /// Inline namespaces won't be reported at all unless the + /// "enable conservative inline namespaces" option is enabled. + Mod { + /// The final name used. + final_name: String, + /// Whether this was originally an anonymous namespace. + /// bindgen will have assigned a name within `final_name`. + anonymous: bool, + /// Whether this is an inline namespace. + inline: bool, + }, + /// A function or method. Function { /// The final name used. @@ -244,7 +269,7 @@ pub enum DiscoveredItem { /// Type to which this method belongs. parent: DiscoveredItemId, - }, // modules, etc. + }, } /// Relevant information about a type to which new derive attributes will be added using diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 322922b463..26c5324f7e 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -627,6 +627,19 @@ impl CodeGenerator for Module { } let name = item.canonical_name(ctx); + + utils::call_discovered_item_callback(ctx, item, || { + DiscoveredItem::Mod { + final_name: if item.id() == item.parent_id() { + "".to_string() // root module + } else { + name.clone() + }, + anonymous: self.name().is_none(), + inline: self.is_inline(), + } + }); + let ident = ctx.rust_ident(name); result.push(if item.id() == ctx.root_module() { quote! { @@ -5198,6 +5211,7 @@ pub(crate) mod utils { use crate::ir::context::TypeId; use crate::ir::function::{Abi, ClangAbi, FunctionSig}; use crate::ir::item::{Item, ItemCanonicalPath}; + use crate::ir::item_kind::ItemKind; use crate::ir::ty::TypeKind; use crate::{args_are_cpp, file_is_cpp}; use std::borrow::Cow; @@ -5939,12 +5953,64 @@ pub(crate) mod utils { file_name, } }); + ctx.options().for_each_callback(|cb| { cb.new_item_found( DiscoveredItemId::new(item.id().as_usize()), discovered_item_creator(), source_location.as_ref(), + find_reportable_parent(ctx, item), ); }); } + + /// Identify a suitable parent, the details of which will have + /// been passed to `ParseCallbacks`. We don't inform + /// [`crate::callbacks::ParseCallbacks::new_item_found`] + /// about everything - notably, not usually inline namespaces - and always + /// want to ensure that the `parent_id` we report within `new_item_found` + /// always corresponds to some other item which we'll have + /// told the client about. This function hops back through the ancestor + /// chain until it finds a reportable ID. + pub(super) fn find_reportable_parent( + ctx: &BindgenContext, + item: &Item, + ) -> Option { + // We choose never to report parents if C++ namespaces are not + // enabled. Sometimes a struct might be within another struct, but + // for now we simply don't report parentage at all. + if !ctx.options().enable_cxx_namespaces { + return None; + } + let mut parent_item = ctx.resolve_item(item.parent_id()); + while !is_reportable_parent(ctx, &parent_item) { + let parent_id = parent_item.parent_id(); + if parent_id == parent_item.id() { + return None; + } + parent_item = ctx.resolve_item(parent_id); + } + if parent_item.id() == item.id() { + // This is itself the root module. + None + } else { + Some(DiscoveredItemId::new(parent_item.id().as_usize())) + } + } + + /// Returns whether a given [`Item`] will have been reported, or will + /// be reported, in [`crate::callbacks::ParseCallbacks::new_item_found`]. + fn is_reportable_parent(ctx: &BindgenContext, item: &Item) -> bool { + match item.kind() { + ItemKind::Module(ref module) => { + !module.is_inline() + || ctx.options().conservative_inline_namespaces + } + ItemKind::Type(t) => match t.kind() { + TypeKind::Comp(..) | TypeKind::Enum(..) => true, + _ => false, + }, + _ => false, + } + } } From bf438d1c7d743b67f0233be5c3a68950866bcc4b Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Fri, 21 Feb 2025 14:20:08 +0000 Subject: [PATCH 7/7] Rustfmt fixes. --- .../tests/parse_callbacks/item_discovery_callback/mod.rs | 8 ++++---- bindgen/codegen/mod.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs index 2f64b502dd..6659be62b9 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs @@ -502,8 +502,8 @@ fn compare_item_info( generated: &ItemCache, expected_filename: &str, ) -> bool { - if std::mem::discriminant(&expected_item.item) - != std::mem::discriminant(&generated_item.0) + if std::mem::discriminant(&expected_item.item) != + std::mem::discriminant(&generated_item.0) { return false; } @@ -800,8 +800,8 @@ pub fn compare_mod_info( unreachable!() }; - if expected_anonymous != generated_anonymous - || *expected_inline != *generated_inline + if expected_anonymous != generated_anonymous || + *expected_inline != *generated_inline { return false; } diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 26c5324f7e..4fdc4aa094 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -6003,8 +6003,8 @@ pub(crate) mod utils { fn is_reportable_parent(ctx: &BindgenContext, item: &Item) -> bool { match item.kind() { ItemKind::Module(ref module) => { - !module.is_inline() - || ctx.options().conservative_inline_namespaces + !module.is_inline() || + ctx.options().conservative_inline_namespaces } ItemKind::Type(t) => match t.kind() { TypeKind::Comp(..) | TypeKind::Enum(..) => true,