From 4c9a01839ce04c1ecc03b740379ff5a70f124a7f Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Wed, 19 Feb 2025 15:32:29 +0000 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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 05/10] 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 06/10] 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 07/10] 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, From 61cdc0028d44fe16baecb35b1317217ea4b30841 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Fri, 21 Feb 2025 14:35:32 +0000 Subject: [PATCH 08/10] Report C++ visibility and other details. This change reports extra C++ information about items: * Whether methods are virtual or pure virtual or neither * Whether a method is a "special member", e.g. a move constructor * Whether a method is defaulted or deleted * C++ visibility (for structs, enums, unions and methods) It builds on top of #3145. A follow up PR should enhance the tests once #3139 is merged. Part of https://github.com/google/autocxx/issues/124 --- .../header_item_discovery.hpp | 10 ++ .../item_discovery_callback/mod.rs | 92 +++++++++++++++++- bindgen/callbacks.rs | 38 ++++++++ bindgen/clang.rs | 15 +++ bindgen/codegen/mod.rs | 20 +++- bindgen/ir/comp.rs | 37 ++++++++ bindgen/ir/enum_ty.rs | 15 ++- bindgen/ir/function.rs | 94 ++++++++++++++++++- bindgen/ir/ty.rs | 6 +- 9 files changed, 315 insertions(+), 12 deletions(-) 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 index 1de8d99e4d..0fb70d9396 100644 --- 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 @@ -2,5 +2,15 @@ class SomeClass { public: + SomeClass() = delete; + SomeClass(const SomeClass&) = default; + SomeClass(SomeClass&&); void named_method(); + virtual void virtual_method(); + virtual void pure_virtual_method() = 0; +private: + void private_method(); +protected: + void protected_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 6659be62b9..63066ab590 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs @@ -7,6 +7,7 @@ use regex::Regex; use bindgen::callbacks::{ DiscoveredItem, DiscoveredItemId, ParseCallbacks, SourceLocation, + SpecialMemberKind, Visibility, }; use bindgen::Builder; @@ -101,6 +102,7 @@ fn test_item_discovery_callback_c() { DiscoveredItem::Struct { original_name: Some("NamedStruct".to_string()), final_name: "NamedStruct".to_string(), + cpp_visibility: Visibility::Public, }, 4, 8, @@ -127,6 +129,7 @@ fn test_item_discovery_callback_c() { DiscoveredItem::Union { original_name: Some("NamedUnion".to_string()), final_name: "NamedUnion".to_string(), + cpp_visibility: Visibility::Public, }, 13, 7, @@ -165,6 +168,7 @@ fn test_item_discovery_callback_c() { ItemExpectations::new( DiscoveredItem::Enum { final_name: "NamedEnum".to_string(), + cpp_visibility: Visibility::Public, }, 24, 6, @@ -178,6 +182,7 @@ fn test_item_discovery_callback_c() { DiscoveredItem::Struct { original_name: None, final_name: "_bindgen_ty_*".to_string(), + cpp_visibility: Visibility::Public, }, 2, 38, @@ -191,6 +196,7 @@ fn test_item_discovery_callback_c() { DiscoveredItem::Union { original_name: None, final_name: "_bindgen_ty_*".to_string(), + cpp_visibility: Visibility::Public, }, 11, 37, @@ -216,7 +222,7 @@ fn test_item_discovery_callback_c() { } #[test] -fn test_item_discovery_callback_cpp() { +fn test_item_discovery_callback_cpp_features() { let expected = ExpectationMap::from([ ( DiscoveredItemId::new(1), @@ -224,6 +230,7 @@ fn test_item_discovery_callback_cpp() { DiscoveredItem::Struct { original_name: Some("SomeClass".to_string()), final_name: "SomeClass".to_string(), + cpp_visibility: Visibility::Public, }, 3, 7, @@ -237,16 +244,57 @@ fn test_item_discovery_callback_cpp() { DiscoveredItem::Method { final_name: "named_method".to_string(), parent: DiscoveredItemId::new(1), + cpp_visibility: Visibility::Public, + cpp_special_member: None, + cpp_virtual: None, + cpp_explicit: None, }, - 5, + 8, 10, - 47, + 144, + None, + ), + ), + ( + DiscoveredItemId::new(48), + ItemExpectations::new( + DiscoveredItem::Method { + final_name: "protected_method".to_string(), + parent: DiscoveredItemId::new(1), + cpp_visibility: Visibility::Protected, + cpp_special_member: None, + cpp_virtual: None, + cpp_explicit: None, + }, + 14, + 10, + 295, + None, + ), + ), + ( + DiscoveredItemId::new(19), + ItemExpectations::new( + DiscoveredItem::Method { + final_name: "new".to_string(), + parent: DiscoveredItemId::new(1), + cpp_visibility: Visibility::Public, + cpp_special_member: Some( + SpecialMemberKind::MoveConstructor, + ), + cpp_virtual: None, + cpp_explicit: None, + }, + 7, + 5, + 111, None, ), ), ]); + test_item_discovery_callback( - "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp", expected, identity); + "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp", expected, |b| b.clang_arg("--std=c++11")); } /// Returns the expectations corresponding to header_item_discovery_with_namespaces.hpp, @@ -361,6 +409,7 @@ fn cpp_expectation_map() -> ExpectationMap { DiscoveredItem::Struct { final_name: "L".to_string(), original_name: Some("L".to_string()), + cpp_visibility: Visibility::Public, }, 25, 12, @@ -374,6 +423,7 @@ fn cpp_expectation_map() -> ExpectationMap { DiscoveredItem::Struct { final_name: "L_M".to_string(), original_name: Some("M".to_string()), + cpp_visibility: Visibility::Public, }, 26, 16, @@ -597,6 +647,7 @@ pub fn compare_struct_info( let DiscoveredItem::Struct { original_name: expected_original_name, final_name: expected_final_name, + cpp_visibility: expected_cpp_visibility, } = expected_item else { unreachable!() @@ -605,6 +656,7 @@ pub fn compare_struct_info( let DiscoveredItem::Struct { original_name: generated_original_name, final_name: generated_final_name, + cpp_visibility: generated_cpp_visibility, } = generated_item else { unreachable!() @@ -614,6 +666,10 @@ pub fn compare_struct_info( return false; } + if expected_cpp_visibility != generated_cpp_visibility { + return false; + } + match (expected_original_name, generated_original_name) { (None, None) => true, (Some(expected_original_name), Some(generated_original_name)) => { @@ -630,6 +686,7 @@ pub fn compare_union_info( let DiscoveredItem::Union { original_name: expected_original_name, final_name: expected_final_name, + cpp_visibility: expected_cpp_visibility, } = expected_item else { unreachable!() @@ -638,6 +695,7 @@ pub fn compare_union_info( let DiscoveredItem::Union { original_name: generated_original_name, final_name: generated_final_name, + cpp_visibility: generated_cpp_visibility, } = generated_item else { unreachable!() @@ -647,6 +705,10 @@ pub fn compare_union_info( return false; } + if expected_cpp_visibility != generated_cpp_visibility { + return false; + } + match (expected_original_name, generated_original_name) { (None, None) => true, (Some(expected_original_name), Some(generated_original_name)) => { @@ -662,6 +724,7 @@ pub fn compare_enum_info( ) -> bool { let DiscoveredItem::Enum { final_name: expected_final_name, + cpp_visibility: expected_cpp_visibility, } = expected_item else { unreachable!() @@ -669,6 +732,7 @@ pub fn compare_enum_info( let DiscoveredItem::Enum { final_name: generated_final_name, + cpp_visibility: generated_cpp_visibility, } = generated_item else { unreachable!() @@ -677,6 +741,11 @@ pub fn compare_enum_info( if !compare_names(expected_final_name, generated_final_name) { return false; } + + if expected_cpp_visibility != generated_cpp_visibility { + return false; + } + true } @@ -755,6 +824,10 @@ pub fn compare_method_info( let DiscoveredItem::Method { final_name: expected_final_name, parent: expected_parent, + cpp_visibility: expected_cpp_visibility, + cpp_special_member: expected_cpp_special_member, + cpp_virtual: expected_cpp_virtual, + cpp_explicit: expected_cpp_explicit, } = expected_item else { unreachable!() @@ -763,12 +836,21 @@ pub fn compare_method_info( let DiscoveredItem::Method { final_name: generated_final_name, parent: generated_parent, + cpp_visibility: generated_cpp_visibility, + cpp_special_member: generated_cpp_special_member, + cpp_virtual: generated_cpp_virtual, + cpp_explicit: generated_cpp_explicit, } = generated_item else { unreachable!() }; - if expected_parent != generated_parent { + if expected_parent != generated_parent + || expected_cpp_explicit != generated_cpp_explicit + || expected_cpp_special_member != generated_cpp_special_member + || expected_cpp_virtual != generated_cpp_virtual + || expected_cpp_visibility != generated_cpp_visibility + { return false; } diff --git a/bindgen/callbacks.rs b/bindgen/callbacks.rs index 5cbc1e2b17..44421f606a 100644 --- a/bindgen/callbacks.rs +++ b/bindgen/callbacks.rs @@ -1,8 +1,11 @@ //! A public API for more fine-grained customization of bindgen behavior. pub use crate::ir::analysis::DeriveTrait; +pub use crate::ir::comp::SpecialMemberKind; pub use crate::ir::derive::CanDerive as ImplementsTrait; pub use crate::ir::enum_ty::{EnumVariantCustomBehavior, EnumVariantValue}; +pub use crate::ir::function::Explicitness; +pub use crate::ir::function::Visibility; pub use crate::ir::int::IntKind; use std::fmt; @@ -208,6 +211,10 @@ pub enum DiscoveredItem { /// The name of the generated binding final_name: String, + + /// Its C++ visibility. [`Visibility::Public`] unless this is nested + /// in another type. + cpp_visibility: Visibility, }, /// Represents a union with its original name in C and its generated binding name @@ -218,6 +225,10 @@ pub enum DiscoveredItem { /// The name of the generated binding final_name: String, + + /// Its C++ visibility. [`Visibility::Public`] unless this is nested + /// in another type. + cpp_visibility: Visibility, }, /// Represents an alias like a typedef @@ -239,6 +250,10 @@ pub enum DiscoveredItem { Enum { /// The final name of the generated binding final_name: String, + + /// Its C++ visibility. [`Visibility::Public`] unless this is nested + /// in another type. + cpp_visibility: Visibility, }, /// A module, representing a C++ namespace. @@ -269,6 +284,20 @@ pub enum DiscoveredItem { /// Type to which this method belongs. parent: DiscoveredItemId, + + /// Its C++ visibility. + cpp_visibility: Visibility, + + /// Whether this is a C++ "special member". + cpp_special_member: Option, + + /// Whether this is a C++ virtual function. + cpp_virtual: Option, + + /// Whether this is a C++ function which has been marked + /// `=default` or `=deleted`. Note that deleted functions aren't + /// normally generated without special bindgen options. + cpp_explicit: Option, }, } @@ -336,6 +365,15 @@ pub struct FieldInfo<'a> { pub field_type_name: Option<&'a str>, } +/// Whether a method is virtual or pure virtual. +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum Virtualness { + /// Not pure virtual. + Virtual, + /// Pure virtual. + PureVirtual, +} + /// Location in the source code. Roughly equivalent to the same type /// within `clang_sys`. #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/bindgen/clang.rs b/bindgen/clang.rs index 04fe3e1538..e97e8b6181 100644 --- a/bindgen/clang.rs +++ b/bindgen/clang.rs @@ -927,6 +927,21 @@ impl Cursor { unsafe { clang_isVirtualBase(self.x) != 0 } } + // Is this cursor's referent a default constructor? + pub fn is_default_constructor(&self) -> bool { + unsafe { clang_CXXConstructor_isDefaultConstructor(self.x) != 0 } + } + + // Is this cursor's referent a copy constructor? + pub fn is_copy_constructor(&self) -> bool { + unsafe { clang_CXXConstructor_isCopyConstructor(self.x) != 0 } + } + + // Is this cursor's referent a move constructor? + pub fn is_move_constructor(&self) -> bool { + unsafe { clang_CXXConstructor_isMoveConstructor(self.x) != 0 } + } + /// Try to evaluate this cursor. pub(crate) fn evaluate(&self) -> Option { EvalResult::new(*self) diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 4fdc4aa094..68f4b714f8 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -21,8 +21,7 @@ use self::struct_layout::StructLayoutTracker; use super::BindgenOptions; use crate::callbacks::{ - AttributeInfo, DeriveInfo, DiscoveredItem, DiscoveredItemId, FieldInfo, - TypeKind as DeriveTypeKind, + AttributeInfo, DeriveInfo, DiscoveredItem, DiscoveredItemId, FieldInfo, TypeKind as DeriveTypeKind, Virtualness }; use crate::codegen::error::Error; use crate::ir::analysis::{HasVtable, Sizedness}; @@ -2499,6 +2498,7 @@ impl CodeGenerator for CompInfo { .name() .map(String::from), final_name: canonical_ident.to_string(), + cpp_visibility: self.visibility(), }, CompKind::Union => DiscoveredItem::Union { original_name: item @@ -2507,6 +2507,7 @@ impl CodeGenerator for CompInfo { .name() .map(String::from), final_name: canonical_ident.to_string(), + cpp_visibility: self.visibility(), }, }); @@ -3074,9 +3075,23 @@ impl Method { method_names.insert(name.clone()); utils::call_discovered_item_callback(ctx, function_item, || { + let cpp_virtual = match function.kind() { + FunctionKind::Function => None, + FunctionKind::Method(method_kind) => if method_kind.is_pure_virtual() { + Some(Virtualness::PureVirtual) + } else if method_kind.is_virtual() { + Some(Virtualness::Virtual) + } else { + None + } + }; DiscoveredItem::Method { parent: parent_id, final_name: name.clone(), + cpp_visibility: function.visibility(), + cpp_special_member: function.special_member(), + cpp_virtual, + cpp_explicit: function.explicitness(), } }); @@ -3788,6 +3803,7 @@ impl CodeGenerator for Enum { utils::call_discovered_item_callback(ctx, item, || { DiscoveredItem::Enum { final_name: name.to_string(), + cpp_visibility: self.visibility, } }); diff --git a/bindgen/ir/comp.rs b/bindgen/ir/comp.rs index 15f9cb4655..15d91328ed 100644 --- a/bindgen/ir/comp.rs +++ b/bindgen/ir/comp.rs @@ -6,6 +6,7 @@ use super::analysis::Sizedness; use super::annotations::Annotations; use super::context::{BindgenContext, FunctionId, ItemId, TypeId, VarId}; use super::dot::DotAttributes; +use super::function::Visibility; use super::item::{IsOpaque, Item}; use super::layout::Layout; use super::template::TemplateParameters; @@ -63,6 +64,14 @@ impl MethodKind { ) } + /// Is this virtual (pure or otherwise?) + pub(crate) fn is_virtual(self) -> bool { + matches!( + self, + MethodKind::Virtual { .. } | MethodKind::VirtualDestructor { .. } + ) + } + /// Is this a pure virtual method? pub(crate) fn is_pure_virtual(self) -> bool { match self { @@ -73,6 +82,23 @@ impl MethodKind { } } +/// The kind of C++ special member. +// TODO: We don't currently cover copy assignment or move assignment operator +// because libclang doesn't provide a way to query for them. +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum SpecialMemberKind { + /// The default constructor. + DefaultConstructor, + /// A copy constructor. + CopyConstructor, + /// A move constructor. + MoveConstructor, + /// A destructor. + Destructor, + /// The assignment operator. + AssignmentOperator, +} + /// A struct representing a C++ method, either static, normal, or virtual. #[derive(Debug)] pub(crate) struct Method { @@ -993,6 +1019,10 @@ pub(crate) struct CompInfo { /// Whether this is a struct or a union. kind: CompKind, + /// The visibility of this struct or union if it was declared inside of + /// another type. Top-level types always have public visibility. + visibility: Visibility, + /// The members of this struct or union. fields: CompFields, @@ -1074,6 +1104,7 @@ impl CompInfo { pub(crate) fn new(kind: CompKind) -> Self { CompInfo { kind, + visibility: Visibility::Public, fields: CompFields::default(), template_params: vec![], methods: vec![], @@ -1193,6 +1224,11 @@ impl CompInfo { } } + /// Returns the visibility of the type. + pub fn visibility(&self) -> Visibility { + self.visibility + } + /// Returns whether we have a too large bitfield unit, in which case we may /// not be able to derive some of the things we should be able to normally /// derive. @@ -1282,6 +1318,7 @@ impl CompInfo { debug!("CompInfo::from_ty({kind:?}, {cursor:?})"); let mut ci = CompInfo::new(kind); + ci.visibility = Visibility::from(cursor.access_specifier()); ci.is_forward_declaration = location.map_or(true, |cur| match cur.kind() { CXCursor_ParmDecl => true, diff --git a/bindgen/ir/enum_ty.rs b/bindgen/ir/enum_ty.rs index 9b08da3bce..3d3b713047 100644 --- a/bindgen/ir/enum_ty.rs +++ b/bindgen/ir/enum_ty.rs @@ -2,6 +2,7 @@ use super::super::codegen::EnumVariation; use super::context::{BindgenContext, TypeId}; +use super::function::Visibility; use super::item::Item; use super::ty::{Type, TypeKind}; use crate::clang; @@ -32,6 +33,10 @@ pub(crate) struct Enum { /// The different variants, with explicit values. variants: Vec, + + /// The visibility of this enum if it was declared inside of + /// another type. Top-level types always have public visibility. + pub(crate) visibility: Visibility, } impl Enum { @@ -39,8 +44,13 @@ impl Enum { pub(crate) fn new( repr: Option, variants: Vec, + visibility: Visibility, ) -> Self { - Enum { repr, variants } + Enum { + repr, + variants, + visibility, + } } /// Get this enumeration's representation. @@ -56,6 +66,7 @@ impl Enum { /// Construct an enumeration from the given Clang type. pub(crate) fn from_ty( ty: &clang::Type, + visibility: Visibility, ctx: &mut BindgenContext, ) -> Result { use clang_sys::*; @@ -147,7 +158,7 @@ impl Enum { } CXChildVisit_Continue }); - Ok(Enum::new(repr, variants)) + Ok(Enum::new(repr, variants, visibility)) } fn is_matching_enum( diff --git a/bindgen/ir/function.rs b/bindgen/ir/function.rs index 83b748a5f4..df1f97912d 100644 --- a/bindgen/ir/function.rs +++ b/bindgen/ir/function.rs @@ -1,6 +1,6 @@ //! Intermediate representation for C/C++ functions and methods. -use super::comp::MethodKind; +use super::comp::{MethodKind, SpecialMemberKind}; use super::context::{BindgenContext, TypeId}; use super::dot::DotAttributes; use super::item::Item; @@ -9,7 +9,9 @@ use super::ty::TypeKind; use crate::callbacks::{ItemInfo, ItemKind}; use crate::clang::{self, ABIKind, Attribute}; use crate::parse::{ClangSubItemParser, ParseError, ParseResult}; -use clang_sys::CXCallingConv; +use clang_sys::{ + CXCallingConv, CX_CXXAccessSpecifier, CX_CXXPrivate, CX_CXXProtected, +}; use quote::TokenStreamExt; use std::io; @@ -70,6 +72,38 @@ pub(crate) enum Linkage { Internal, } +/// C++ visibility. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum Visibility { + /// `public` visibility. + Public, + /// `protected` visibility. + Protected, + /// `private` visibility. + Private, +} + +impl From for Visibility { + fn from(access_specifier: CX_CXXAccessSpecifier) -> Self { + if access_specifier == CX_CXXPrivate { + Visibility::Private + } else if access_specifier == CX_CXXProtected { + Visibility::Protected + } else { + Visibility::Public + } + } +} + +/// Whether a C++ method has been explicitly defaulted or deleted. +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum Explicitness { + /// Defaulted function, i.e. `=default` + Defaulted, + /// Deleted function, i.e. `=delete` + Deleted, +} + /// A function declaration, with a signature, arguments, and argument names. /// /// The argument names vector must be the same length as the ones in the @@ -93,6 +127,15 @@ pub(crate) struct Function { /// The linkage of the function. linkage: Linkage, + + /// C++ Special member kind, if applicable + special_member: Option, + + /// Whether it is private + visibility: Visibility, + + // Whether it is `=delete` or `=default` + explicitness: Option, } impl Function { @@ -104,6 +147,9 @@ impl Function { signature: TypeId, kind: FunctionKind, linkage: Linkage, + special_member: Option, + visibility: Visibility, + explicitness: Option, ) -> Self { Function { name, @@ -112,6 +158,9 @@ impl Function { signature, kind, linkage, + special_member, + visibility, + explicitness, } } @@ -144,6 +193,22 @@ impl Function { pub(crate) fn linkage(&self) -> Linkage { self.linkage } + + /// Get this function's C++ special member kind. + pub fn special_member(&self) -> Option { + self.special_member + } + + /// Whether it is private, protected or public + pub fn visibility(&self) -> Visibility { + self.visibility + } + + /// Whether this is a function that's been deleted (=delete) + /// or defaulted (=default) + pub fn explicitness(&self) -> Option { + self.explicitness + } } impl DotAttributes for Function { @@ -736,6 +801,8 @@ impl ClangSubItemParser for Function { return Err(ParseError::Continue); } + let visibility = Visibility::from(cursor.access_specifier()); + let linkage = cursor.linkage(); let linkage = match linkage { CXLinkage_External | CXLinkage_UniqueExternal => Linkage::External, @@ -803,6 +870,26 @@ impl ClangSubItemParser for Function { }) }); + let special_member = if cursor.is_default_constructor() { + Some(SpecialMemberKind::DefaultConstructor) + } else if cursor.is_copy_constructor() { + Some(SpecialMemberKind::CopyConstructor) + } else if cursor.is_move_constructor() { + Some(SpecialMemberKind::MoveConstructor) + } else if cursor.kind() == CXCursor_Destructor { + Some(SpecialMemberKind::Destructor) + } else { + None + }; + + let explicitness = if cursor.is_deleted_function() { + Some(Explicitness::Deleted) + } else if cursor.is_defaulted_function() { + Some(Explicitness::Defaulted) + } else { + None + }; + let function = Self::new( name.clone(), mangled_name, @@ -810,6 +897,9 @@ impl ClangSubItemParser for Function { sig, kind, linkage, + special_member, + visibility, + explicitness, ); Ok(ParseResult::New(function, Some(cursor))) diff --git a/bindgen/ir/ty.rs b/bindgen/ir/ty.rs index 2589a56fca..42c25cea8f 100644 --- a/bindgen/ir/ty.rs +++ b/bindgen/ir/ty.rs @@ -13,6 +13,7 @@ use super::template::{ }; use super::traversal::{EdgeKind, Trace, Tracer}; use crate::clang::{self, Cursor}; +use crate::ir::function::Visibility; use crate::parse::{ParseError, ParseResult}; use std::borrow::Cow; use std::io; @@ -1086,7 +1087,10 @@ impl Type { } } CXType_Enum => { - let enum_ = Enum::from_ty(ty, ctx).expect("Not an enum?"); + let visibility = + Visibility::from(cursor.access_specifier()); + let enum_ = Enum::from_ty(ty, visibility, ctx) + .expect("Not an enum?"); if !is_anonymous { let pretty_name = ty.spelling(); From 37757eebae32829055edb1f8ebcd648b38c36400 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Fri, 21 Feb 2025 17:41:08 +0000 Subject: [PATCH 09/10] Tweaks to match CI rustfmt --- .../parse_callbacks/item_discovery_callback/mod.rs | 10 +++++----- bindgen/codegen/mod.rs | 14 ++++++++------ 2 files changed, 13 insertions(+), 11 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 63066ab590..ead1663a2c 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs @@ -845,11 +845,11 @@ pub fn compare_method_info( unreachable!() }; - if expected_parent != generated_parent - || expected_cpp_explicit != generated_cpp_explicit - || expected_cpp_special_member != generated_cpp_special_member - || expected_cpp_virtual != generated_cpp_virtual - || expected_cpp_visibility != generated_cpp_visibility + if expected_parent != generated_parent || + expected_cpp_explicit != generated_cpp_explicit || + expected_cpp_special_member != generated_cpp_special_member || + expected_cpp_virtual != generated_cpp_virtual || + expected_cpp_visibility != generated_cpp_visibility { return false; } diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 68f4b714f8..0b7989d1d8 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -3077,12 +3077,14 @@ impl Method { utils::call_discovered_item_callback(ctx, function_item, || { let cpp_virtual = match function.kind() { FunctionKind::Function => None, - FunctionKind::Method(method_kind) => if method_kind.is_pure_virtual() { - Some(Virtualness::PureVirtual) - } else if method_kind.is_virtual() { - Some(Virtualness::Virtual) - } else { - None + FunctionKind::Method(method_kind) => { + if method_kind.is_pure_virtual() { + Some(Virtualness::PureVirtual) + } else if method_kind.is_virtual() { + Some(Virtualness::Virtual) + } else { + None + } } }; DiscoveredItem::Method { From 11d76b0dfc3c997989e9b646933f947382bd97ba Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Fri, 21 Feb 2025 17:55:06 +0000 Subject: [PATCH 10/10] Another clippy fix. --- bindgen/codegen/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 0b7989d1d8..ed92cee1d6 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -21,7 +21,8 @@ use self::struct_layout::StructLayoutTracker; use super::BindgenOptions; use crate::callbacks::{ - AttributeInfo, DeriveInfo, DiscoveredItem, DiscoveredItemId, FieldInfo, TypeKind as DeriveTypeKind, Virtualness + AttributeInfo, DeriveInfo, DiscoveredItem, DiscoveredItemId, FieldInfo, + TypeKind as DeriveTypeKind, Virtualness, }; use crate::codegen::error::Error; use crate::ir::analysis::{HasVtable, Sizedness};