diff options
author | Nick Fitzgerald <fitzgen@gmail.com> | 2017-09-29 12:16:36 -0700 |
---|---|---|
committer | Nick Fitzgerald <fitzgen@gmail.com> | 2017-09-29 12:20:49 -0700 |
commit | 64d73c28600686dff5a54d2a1afb737e291f9e08 (patch) | |
tree | 941bfb20f260351e635bbcdd0f4df3fb78ac6366 | |
parent | c160b20218ecd85b2dfc141485b85beec88bac4f (diff) |
ir: Prefer using known semantic parents
When choosing a parent ID for a type that we are parsing, prefer known semantic
parents over the provided parent ID. It seems like we shouldn't even be passing
explicit parent IDs around (they're often buggy), and instead should expand the
`known_semantic_parent` infrastructure, but I'll leave that to some future work.
Fixes #1048
-rw-r--r-- | src/ir/comp.rs | 1 | ||||
-rw-r--r-- | src/ir/context.rs | 22 | ||||
-rw-r--r-- | src/ir/item.rs | 3 | ||||
-rw-r--r-- | tests/expectations/tests/sentry-defined-multiple-times.rs | 418 | ||||
-rw-r--r-- | tests/headers/sentry-defined-multiple-times.hpp | 85 |
5 files changed, 519 insertions, 10 deletions
diff --git a/src/ir/comp.rs b/src/ir/comp.rs index fc17ab8f..e1b2a1f0 100644 --- a/src/ir/comp.rs +++ b/src/ir/comp.rs @@ -1152,6 +1152,7 @@ impl CompInfo { let inner = Item::parse(cur, Some(potential_id), ctx) .expect("Inner ClassDecl"); + assert_eq!(ctx.resolve_item(inner).parent_id(), potential_id); ci.inner_types.push(inner); diff --git a/src/ir/context.rs b/src/ir/context.rs index b540d152..5b6b46ef 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -751,25 +751,29 @@ impl BindgenContext { let typerefs = self.collect_typerefs(); for (id, ty, loc, parent_id) in typerefs { - let _resolved = - { - let resolved = Item::from_ty(&ty, loc, parent_id, self) + let _resolved = { + let resolved = Item::from_ty(&ty, loc, parent_id, self) .unwrap_or_else(|_| { warn!("Could not resolve type reference, falling back \ to opaque blob"); Item::new_opaque_type(self.next_item_id(), &ty, self) }); - let item = self.items.get_mut(&id).unwrap(); - *item.kind_mut().as_type_mut().unwrap().kind_mut() = - TypeKind::ResolvedTypeRef(resolved); - resolved - }; + let item = self.items.get_mut(&id).unwrap(); + *item.kind_mut().as_type_mut().unwrap().kind_mut() = + TypeKind::ResolvedTypeRef(resolved); + + resolved + }; // Something in the STL is trolling me. I don't need this assertion // right now, but worth investigating properly once this lands. // // debug_assert!(self.items.get(&resolved).is_some(), "How?"); + // + // if let Some(parent_id) = parent_id { + // assert_eq!(self.items[&resolved].parent_id(), parent_id); + // } } } @@ -953,7 +957,7 @@ impl BindgenContext { self.compute_bitfield_units(); self.process_replacements(); } - + self.deanonymize_fields(); // And assert once again, because resolving type refs and processing diff --git a/src/ir/item.rs b/src/ir/item.rs index 8a5a143b..955793c1 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -1216,7 +1216,8 @@ impl ClangItemParser for Item { ctx, )); } - parent_id.or_else(|| ctx.known_semantic_parent(definition)) + ctx.known_semantic_parent(definition) + .or(parent_id) .unwrap_or(ctx.current_module()) } None => relevant_parent_id, diff --git a/tests/expectations/tests/sentry-defined-multiple-times.rs b/tests/expectations/tests/sentry-defined-multiple-times.rs new file mode 100644 index 00000000..7d105c2a --- /dev/null +++ b/tests/expectations/tests/sentry-defined-multiple-times.rs @@ -0,0 +1,418 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] + + +#[allow(non_snake_case, non_camel_case_types, non_upper_case_globals)] +pub mod root { + #[allow(unused_imports)] + use self::super::root; + pub mod whatever { + #[allow(unused_imports)] + use self::super::super::root; + #[repr(C)] + #[derive(Debug, Default, Copy, Clone)] + pub struct Wrapper { + pub _address: u8, + } + #[repr(C)] + #[derive(Debug, Default, Copy, Clone)] + pub struct Wrapper_sentry { + pub i_am_wrapper_sentry: ::std::os::raw::c_int, + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct sentry { + pub i_am_plain_sentry: bool, + } + #[test] + fn bindgen_test_layout_sentry() { + assert_eq!( + ::std::mem::size_of::<sentry>(), + 1usize, + concat!("Size of: ", stringify!(sentry)) + ); + assert_eq!( + ::std::mem::align_of::<sentry>(), + 1usize, + concat!("Alignment of ", stringify!(sentry)) + ); + assert_eq!( + unsafe { &(*(0 as *const sentry)).i_am_plain_sentry as *const _ as usize }, + 0usize, + concat!( + "Alignment of field: ", + stringify!(sentry), + "::", + stringify!(i_am_plain_sentry) + ) + ); + } + impl Clone for sentry { + fn clone(&self) -> Self { + *self + } + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct NotTemplateWrapper { + pub _address: u8, + } + #[test] + fn bindgen_test_layout_NotTemplateWrapper() { + assert_eq!( + ::std::mem::size_of::<NotTemplateWrapper>(), + 1usize, + concat!("Size of: ", stringify!(NotTemplateWrapper)) + ); + assert_eq!( + ::std::mem::align_of::<NotTemplateWrapper>(), + 1usize, + concat!("Alignment of ", stringify!(NotTemplateWrapper)) + ); + } + impl Clone for NotTemplateWrapper { + fn clone(&self) -> Self { + *self + } + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct NotTemplateWrapper_sentry { + pub i_am_not_template_wrapper_sentry: ::std::os::raw::c_char, + } + #[test] + fn bindgen_test_layout_NotTemplateWrapper_sentry() { + assert_eq!( + ::std::mem::size_of::<NotTemplateWrapper_sentry>(), + 1usize, + concat!("Size of: ", stringify!(NotTemplateWrapper_sentry)) + ); + assert_eq!( + ::std::mem::align_of::<NotTemplateWrapper_sentry>(), + 1usize, + concat!("Alignment of ", stringify!(NotTemplateWrapper_sentry)) + ); + assert_eq!( + unsafe { + &(*(0 as *const NotTemplateWrapper_sentry)).i_am_not_template_wrapper_sentry as + *const _ as usize + }, + 0usize, + concat!( + "Alignment of field: ", + stringify!(NotTemplateWrapper_sentry), + "::", + stringify!(i_am_not_template_wrapper_sentry) + ) + ); + } + impl Clone for NotTemplateWrapper_sentry { + fn clone(&self) -> Self { + *self + } + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct InlineNotTemplateWrapper { + pub _address: u8, + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct InlineNotTemplateWrapper_sentry { + pub i_am_inline_not_template_wrapper_sentry: bool, + } + #[test] + fn bindgen_test_layout_InlineNotTemplateWrapper_sentry() { + assert_eq!( + ::std::mem::size_of::<InlineNotTemplateWrapper_sentry>(), + 1usize, + concat!("Size of: ", stringify!(InlineNotTemplateWrapper_sentry)) + ); + assert_eq!( + ::std::mem::align_of::<InlineNotTemplateWrapper_sentry>(), + 1usize, + concat!("Alignment of ", stringify!(InlineNotTemplateWrapper_sentry)) + ); + assert_eq!( + unsafe { + &(*(0 as *const InlineNotTemplateWrapper_sentry)) + .i_am_inline_not_template_wrapper_sentry as *const _ as + usize + }, + 0usize, + concat!( + "Alignment of field: ", + stringify!(InlineNotTemplateWrapper_sentry), + "::", + stringify!(i_am_inline_not_template_wrapper_sentry) + ) + ); + } + impl Clone for InlineNotTemplateWrapper_sentry { + fn clone(&self) -> Self { + *self + } + } + #[test] + fn bindgen_test_layout_InlineNotTemplateWrapper() { + assert_eq!( + ::std::mem::size_of::<InlineNotTemplateWrapper>(), + 1usize, + concat!("Size of: ", stringify!(InlineNotTemplateWrapper)) + ); + assert_eq!( + ::std::mem::align_of::<InlineNotTemplateWrapper>(), + 1usize, + concat!("Alignment of ", stringify!(InlineNotTemplateWrapper)) + ); + } + impl Clone for InlineNotTemplateWrapper { + fn clone(&self) -> Self { + *self + } + } + #[repr(C)] + #[derive(Debug, Default, Copy, Clone)] + pub struct InlineTemplateWrapper { + pub _address: u8, + } + #[repr(C)] + #[derive(Debug, Default, Copy, Clone)] + pub struct InlineTemplateWrapper_sentry { + pub i_am_inline_template_wrapper_sentry: ::std::os::raw::c_int, + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct OuterDoubleWrapper { + pub _address: u8, + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct OuterDoubleWrapper_InnerDoubleWrapper { + pub _address: u8, + } + #[test] + fn bindgen_test_layout_OuterDoubleWrapper_InnerDoubleWrapper() { + assert_eq!( + ::std::mem::size_of::<OuterDoubleWrapper_InnerDoubleWrapper>(), + 1usize, + concat!( + "Size of: ", + stringify!(OuterDoubleWrapper_InnerDoubleWrapper) + ) + ); + assert_eq!( + ::std::mem::align_of::<OuterDoubleWrapper_InnerDoubleWrapper>(), + 1usize, + concat!( + "Alignment of ", + stringify!(OuterDoubleWrapper_InnerDoubleWrapper) + ) + ); + } + impl Clone for OuterDoubleWrapper_InnerDoubleWrapper { + fn clone(&self) -> Self { + *self + } + } + #[test] + fn bindgen_test_layout_OuterDoubleWrapper() { + assert_eq!( + ::std::mem::size_of::<OuterDoubleWrapper>(), + 1usize, + concat!("Size of: ", stringify!(OuterDoubleWrapper)) + ); + assert_eq!( + ::std::mem::align_of::<OuterDoubleWrapper>(), + 1usize, + concat!("Alignment of ", stringify!(OuterDoubleWrapper)) + ); + } + impl Clone for OuterDoubleWrapper { + fn clone(&self) -> Self { + *self + } + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct OuterDoubleWrapper_InnerDoubleWrapper_sentry { + pub i_am_double_wrapper_sentry: ::std::os::raw::c_int, + } + #[test] + fn bindgen_test_layout_OuterDoubleWrapper_InnerDoubleWrapper_sentry() { + assert_eq!( + ::std::mem::size_of::<OuterDoubleWrapper_InnerDoubleWrapper_sentry>(), + 4usize, + concat!( + "Size of: ", + stringify!(OuterDoubleWrapper_InnerDoubleWrapper_sentry) + ) + ); + assert_eq!( + ::std::mem::align_of::<OuterDoubleWrapper_InnerDoubleWrapper_sentry>(), + 4usize, + concat!( + "Alignment of ", + stringify!(OuterDoubleWrapper_InnerDoubleWrapper_sentry) + ) + ); + assert_eq!( + unsafe { + &(*(0 as *const OuterDoubleWrapper_InnerDoubleWrapper_sentry)) + .i_am_double_wrapper_sentry as *const _ as usize + }, + 0usize, + concat!( + "Alignment of field: ", + stringify!(OuterDoubleWrapper_InnerDoubleWrapper_sentry), + "::", + stringify!(i_am_double_wrapper_sentry) + ) + ); + } + impl Clone for OuterDoubleWrapper_InnerDoubleWrapper_sentry { + fn clone(&self) -> Self { + *self + } + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct OuterDoubleInlineWrapper { + pub _address: u8, + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct OuterDoubleInlineWrapper_InnerDoubleInlineWrapper { + pub _address: u8, + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct OuterDoubleInlineWrapper_InnerDoubleInlineWrapper_sentry { + pub i_am_double_wrapper_inline_sentry: ::std::os::raw::c_int, + } + #[test] + fn bindgen_test_layout_OuterDoubleInlineWrapper_InnerDoubleInlineWrapper_sentry() { + assert_eq!( + ::std::mem::size_of::<OuterDoubleInlineWrapper_InnerDoubleInlineWrapper_sentry>(), + 4usize, + concat!( + "Size of: ", + stringify!(OuterDoubleInlineWrapper_InnerDoubleInlineWrapper_sentry) + ) + ); + assert_eq!( + ::std::mem::align_of::<OuterDoubleInlineWrapper_InnerDoubleInlineWrapper_sentry>(), + 4usize, + concat!( + "Alignment of ", + stringify!(OuterDoubleInlineWrapper_InnerDoubleInlineWrapper_sentry) + ) + ); + assert_eq!( + unsafe { + &(*(0 as *const OuterDoubleInlineWrapper_InnerDoubleInlineWrapper_sentry)) + .i_am_double_wrapper_inline_sentry as *const _ as usize + }, + 0usize, + concat!( + "Alignment of field: ", + stringify!(OuterDoubleInlineWrapper_InnerDoubleInlineWrapper_sentry), + "::", + stringify!(i_am_double_wrapper_inline_sentry) + ) + ); + } + impl Clone for OuterDoubleInlineWrapper_InnerDoubleInlineWrapper_sentry { + fn clone(&self) -> Self { + *self + } + } + #[test] + fn bindgen_test_layout_OuterDoubleInlineWrapper_InnerDoubleInlineWrapper() { + assert_eq!( + ::std::mem::size_of::<OuterDoubleInlineWrapper_InnerDoubleInlineWrapper>(), + 1usize, + concat!( + "Size of: ", + stringify!(OuterDoubleInlineWrapper_InnerDoubleInlineWrapper) + ) + ); + assert_eq!( + ::std::mem::align_of::<OuterDoubleInlineWrapper_InnerDoubleInlineWrapper>(), + 1usize, + concat!( + "Alignment of ", + stringify!(OuterDoubleInlineWrapper_InnerDoubleInlineWrapper) + ) + ); + } + impl Clone for OuterDoubleInlineWrapper_InnerDoubleInlineWrapper { + fn clone(&self) -> Self { + *self + } + } + #[test] + fn bindgen_test_layout_OuterDoubleInlineWrapper() { + assert_eq!( + ::std::mem::size_of::<OuterDoubleInlineWrapper>(), + 1usize, + concat!("Size of: ", stringify!(OuterDoubleInlineWrapper)) + ); + assert_eq!( + ::std::mem::align_of::<OuterDoubleInlineWrapper>(), + 1usize, + concat!("Alignment of ", stringify!(OuterDoubleInlineWrapper)) + ); + } + impl Clone for OuterDoubleInlineWrapper { + fn clone(&self) -> Self { + *self + } + } + } + #[repr(C)] + #[derive(Debug, Default, Copy, Clone)] + pub struct OutsideNamespaceWrapper { + pub _address: u8, + } + #[repr(C)] + #[derive(Debug, Default, Copy, Clone)] + pub struct OutsideNamespaceWrapper_sentry { + pub i_am_outside_namespace_wrapper_sentry: ::std::os::raw::c_int, + } + #[repr(C)] + #[derive(Debug, Default, Copy)] + pub struct sentry { + pub i_am_outside_namespace_sentry: ::std::os::raw::c_int, + } + #[test] + fn bindgen_test_layout_sentry() { + assert_eq!( + ::std::mem::size_of::<sentry>(), + 4usize, + concat!("Size of: ", stringify!(sentry)) + ); + assert_eq!( + ::std::mem::align_of::<sentry>(), + 4usize, + concat!("Alignment of ", stringify!(sentry)) + ); + assert_eq!( + unsafe { &(*(0 as *const sentry)).i_am_outside_namespace_sentry as *const _ as usize }, + 0usize, + concat!( + "Alignment of field: ", + stringify!(sentry), + "::", + stringify!(i_am_outside_namespace_sentry) + ) + ); + } + impl Clone for sentry { + fn clone(&self) -> Self { + *self + } + } +} diff --git a/tests/headers/sentry-defined-multiple-times.hpp b/tests/headers/sentry-defined-multiple-times.hpp new file mode 100644 index 00000000..d44837d0 --- /dev/null +++ b/tests/headers/sentry-defined-multiple-times.hpp @@ -0,0 +1,85 @@ +// bindgen-flags: --enable-cxx-namespaces -- -std=c++11 + +// `Wrapper::sentry` and `sentry` should be emitted as `Wrapper_sentry` and +// `sentry` respectively, but instead `Wrapper::sentry` is named just `sentry` +// which leads to compilation errors. +// +// Note: if there is no namespace, then we don't run into problems. Similarly, +// making the `Wrapper::sentry` definition inline in `Wrapper`, rather than +// declared inline with an out of line definition, makes the problem go away as +// well. + +namespace whatever { + template <typename, typename> + class Wrapper { + // Declaration of Wrapper::sentry + class sentry; + }; + + // Definition of Wrapper::sentry + template <typename f, typename h> + class Wrapper<f, h>::sentry { + int i_am_wrapper_sentry; + }; + + class sentry { + bool i_am_plain_sentry; + }; + + // Ok, that was the original bug report. While we're here, let's just try + // lots of different things that could go wrong and make sure we handle them + // right. + + class NotTemplateWrapper { + class sentry; + }; + + class NotTemplateWrapper::sentry { + char i_am_not_template_wrapper_sentry; + }; + + class InlineNotTemplateWrapper { + class sentry { + bool i_am_inline_not_template_wrapper_sentry; + }; + }; + + template <typename, typename> + class InlineTemplateWrapper { + class sentry { + int i_am_inline_template_wrapper_sentry; + }; + }; + + class OuterDoubleWrapper { + class InnerDoubleWrapper { + class sentry; + }; + }; + + class OuterDoubleWrapper::InnerDoubleWrapper::sentry { + int i_am_double_wrapper_sentry; + }; + + class OuterDoubleInlineWrapper { + class InnerDoubleInlineWrapper { + class sentry { + int i_am_double_wrapper_inline_sentry; + }; + }; + }; +} + +template <typename, typename> +class OutsideNamespaceWrapper { + class sentry; +}; + +template <typename f, typename h> +class OutsideNamespaceWrapper<f, h>::sentry { + int i_am_outside_namespace_wrapper_sentry; +}; + +class sentry { + int i_am_outside_namespace_sentry; +}; |