Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing instantiation with argument of a type that we didn't generate bindings for #769

Closed
fitzgen opened this issue Jun 19, 2017 · 4 comments

Comments

@fitzgen
Copy link
Member

fitzgen commented Jun 19, 2017

Input C/C++ Header

namespace JS {
template <typename a> class Rooted { a b; };
class AutoValueVector : Rooted<int> {
  using Vec = int;
  using c = Rooted<Vec>;
};
}

Bindgen Invocation

Some complicated thing from SM bindings. Will see what more minimal flags required are and report back.

Actual Results

/* automatically generated by rust-bindgen */

#![feature(const_fn)]
pub use self::root::*;
unsafe impl Sync for JSClass {}
unsafe impl Sync for JSFunctionSpec {}
unsafe impl Sync for JSNativeWrapper {}
unsafe impl Sync for JSPropertySpec {}
unsafe impl Sync for JSTypedMethodJitInfo {}

#[allow(non_snake_case, non_camel_case_types, non_upper_case_globals)]
pub mod root {
    #[allow(unused_imports)]
    use self::super::root;
    pub mod JS {
        #[allow(unused_imports)]
        use self::super::super::root;
        #[repr(C)]
        #[derive(Debug, Copy, Clone)]
        pub struct Rooted<a> {
            pub b: a,
            pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<a>>,
        }
    }
    #[test]
    fn __bindgen_test_layout_Rooted_instantiation_1() {
        assert_eq!(::std::mem::size_of::<root::JS::Rooted<::std::os::raw::c_int>>()
                   , 4usize , concat ! (
                   "Size of template specialization: " , stringify ! (
                   root::JS::Rooted<::std::os::raw::c_int> ) ));
        assert_eq!(::std::mem::align_of::<root::JS::Rooted<::std::os::raw::c_int>>()
                   , 4usize , concat ! (
                   "Alignment of template specialization: " , stringify ! (
                   root::JS::Rooted<::std::os::raw::c_int> ) ));
    }
    #[test]
    fn __bindgen_test_layout_Rooted_instantiation_2() {
        assert_eq!(::std::mem::size_of::<root::JS::Rooted<root::JS::AutoValueVector_Vec>>()
                   , 4usize , concat ! (
                   "Size of template specialization: " , stringify ! (
                   root::JS::Rooted<root::JS::AutoValueVector_Vec> ) ));
        assert_eq!(::std::mem::align_of::<root::JS::Rooted<root::JS::AutoValueVector_Vec>>()
                   , 4usize , concat ! (
                   "Alignment of template specialization: " , stringify ! (
                   root::JS::Rooted<root::JS::AutoValueVector_Vec> ) ));
    }
}

And then this fails to compile because we don't emit any definition for AutoValueVector_Vec:

error[E0412]: cannot find type `AutoValueVector_Vec` in module `root::JS`
  --> js.rs:42:70
   |
42 |         assert_eq!(::std::mem::align_of::<root::JS::Rooted<root::JS::AutoValueVector_Vec>>()
   |                                                                      ^^^^^^^^^^^^^^^^^^^ not found in `root::JS`

Expected Results

We probably should just avoid emitting tests of any instantiation involving non-whitelisted things.

@fitzgen
Copy link
Member Author

fitzgen commented Jun 19, 2017

It seems that --enable-cxx-namespaces --whitelist-type JS::Rooted is enough to trigger this.

@fitzgen
Copy link
Member Author

fitzgen commented Jun 19, 2017

Slightly more reduced test case:

// bindgen-flags: --enable-cxx-namespaces --whitelist-type Rooted

template <typename T>
class Rooted {
    T member;
};

class AutoValueVector : Rooted<int> {
    using IntAlias = int;
    using RootedIntAlias = Rooted<IntAlias>;
};

@fitzgen
Copy link
Member Author

fitzgen commented Jun 19, 2017

We probably should just avoid emitting tests of any instantiation involving non-whitelisted things.

Alternatively, we should consider the argument type whitelisted, and should emit bindings for it. I think that this is what is supposed to be happening, but we are collapsing a type alias away (or maybe libclang is doing this before giving it to us...).

@fitzgen
Copy link
Member Author

fitzgen commented Jun 19, 2017

Ok, here's what's going on: the alias is whitelisted, but because its parent is a struct, and not a module, and its parent struct is not whitelisted, we don't ever add it as a child of a module in BindgenContext::add_item and therefore don't ever generate a definition for it in the codegen phase.

fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 19, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 19, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 19, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 19, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 19, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 20, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 20, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 20, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 20, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
fitzgen added a commit to fitzgen/rust-bindgen that referenced this issue Jun 20, 2017
Previously, if an item's parent was not a module (eg a nested class definition
whose parent it the outer class definition) and the parent was not whitelisted
but the item was transitively whitelisted, then we could generate uses of the
item without emitting any definition for it. This could happen because we were
relying on the outer type calling for code generation on its inner types, but
that relies on us doing code generation for the outer type, which won't happen
if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's
children list, and so will be code generated even if their parent is not
whitelisted.

Fixes rust-lang#769
bors-servo pushed a commit that referenced this issue Jun 20, 2017
Ensure that every item is in some module's children list

Previously, if an item's parent was not a module (eg a nested class definition whose parent it the outer class definition) and the parent was not whitelisted but the item was transitively whitelisted, then we could generate uses of the item without emitting any definition for it. This could happen because we were relying on the outer type calling for code generation on its inner types, but that relies on us doing code generation for the outer type, which won't happen if the outer type is not whitelisted.

This commit avoids this gotcha by ensuring that all items end up in a module's children list, and so will be code generated even if their parent is not whitelisted.

This does have the downside of changing the relative order of some of the emitted code, and so this has a big diff (as will the next bindgen update for downstream dependencies) but I actually think the newer order makes more sense, for what that is worth.

Fixes #769

r? @emilio
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant