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

Option to make whitelisting not recursive. #429

Closed
jethrogb opened this issue Jan 24, 2017 · 13 comments
Closed

Option to make whitelisting not recursive. #429

jethrogb opened this issue Jan 24, 2017 · 13 comments

Comments

@jethrogb
Copy link
Contributor

jethrogb commented Jan 24, 2017

Another follow-up from #21. I'm using

.whitelisted_type("mbedtls_.*")
.whitelisted_function("mbedtls_.*")
.whitelisted_var("mbedtls_.*")
.whitelisted_var("MBEDTLS_.*")

but several libc types are included in the output anyway:

pub type __off_t;
pub type __off64_t;
pub type __time_t;
pub struct _IO_FILE;
pub use self::_IO_FILE as FILE;
pub type _IO_lock_t;
pub struct _IO_marker;
pub type Byte;
pub type uInt;
pub type uLong;
pub type Bytef;
pub type voidpf;
pub type alloc_func;
pub type free_func;
pub struct internal_state;
pub struct z_stream_s;
pub use self::z_stream_s as z_stream;
pub type time_t = __time_t;
pub struct __pthread_internal_list;
pub use self::__pthread_internal_list as __pthread_list_t;
pub union _bindgen_ty_14;
pub struct _bindgen_ty_14___pthread_mutex_s;
pub use self::_bindgen_ty_14 as pthread_mutex_t;

I want bindgen to only output definitions in my project's header files, as in 0.19's match_pat. I will depend on other -sys crates to provide the correct types for those defined outside my project's headers. This is obviously the way it has to be if you want to pass the types around to different -sys crates.

This was referenced Jan 24, 2017
@emilio
Copy link
Contributor

emilio commented Jan 24, 2017

That means that those types are used transitively by the functions you've whitelisted. If you want to make some of those opaque, you can make so with opaque_type. If you want just to avoid generating definitions for those, you can use blacklist_type.

I believe what you want is something like opaque_type(".*").whitelisted_type("mbedtls_.*"). Does that work for you?

@jethrogb
Copy link
Contributor Author

jethrogb commented Jan 24, 2017

Unfortunately not. Then, the types I don't want are still included as u8-array definitions.

Also, there seems to be a bug. Without opaque_type I get:

pub struct _bindgen_ty_6 {
    pub s: ::types::raw_types::c_int,
    pub n: usize,
    pub p: *mut mbedtls_mpi_uint,
}
pub use self::_bindgen_ty_6 as mbedtls_mpi;
pub struct _bindgen_ty_7(pub u32);
pub use self::_bindgen_ty_7 as mbedtls_md_type_t;

With opaque_type as you suggested, I get:

pub type mbedtls_mpi = [u64; 3usize];
pub use self::u32 as mbedtls_md_type_t;

I tried whitelisting _bindgen_ty_.* as well, but that doesn't fix this bug. I don't think I can specify a negative lookbehind for mbedtls_ with Rust regexes.

@emilio
Copy link
Contributor

emilio commented Jan 24, 2017

yep, that last self::u32 thing is a bug.

In any case, yeah, I guess we can make whitelisting not recursive (i.e., do not whitelist everything recursively from a whitelisted type). But I think it's easy to understand why that's the default behavior.

@emilio emilio changed the title Whitelist isn't working / match_pat missing Option to make whitelisting not recursive. Jan 24, 2017
@jethrogb
Copy link
Contributor Author

Yes, I think whitelisting non-recursively would fix this.

I don't think your previous answer would be a good solution? I'm not sure if I understand exactly what you mean. I want my types to be defined in full, but outside types to come from outside.

@emilio
Copy link
Contributor

emilio commented Jan 24, 2017

Yes, the solution here is whitelisting non-recursively.

@emilio
Copy link
Contributor

emilio commented Jan 24, 2017

Or alternatively, adding a set of "externally defined types", but that sounds trickier if it can just be fixed this way.

@dimbleby
Copy link
Contributor

dimbleby commented Jan 24, 2017

Especially with the 'but outside types come from outside' bit, this overlaps the first bullet of #424 ("WIBNI I could tell bindgen where to get a type from").

I currently explicitly blacklist externally defined types, but also would be happy for non-recursive whitelisting to save me the trouble.

@fitzgen
Copy link
Member

fitzgen commented Jan 24, 2017

We emit bindings for the transitive closure of whitelisted items because otherwise we would emit bindings referring to types that do not exist and end up with errors when compiling the bindings.

If we can successfully avoid emitting bindings for an item in our computed whitelisting transitive closure set, then it is a bug in our computation of that set (ie it shouldn't be emitted because it is not whitelisted itself nor used by a whitelisted item).

But it doesn't make sense to "whitelist non-recursively", that leads to emitting bindings with references to types that aren't defined.

@emilio
Copy link
Contributor

emilio commented Jan 24, 2017

@fitzgen as far as I understand it, that's exactly the point, not defining some types so they can be imported manually :)

@jethrogb
Copy link
Contributor Author

Emitting bindings with "non-existing types" is exactly the point. The types do exist. They're just defined in another crate.

@fitzgen
Copy link
Member

fitzgen commented Jan 24, 2017

Isn't blacklisting the specific types we want to leave out from the bindings the proper solution then?

@jethrogb
Copy link
Contributor Author

No, a whitelisting approach makes more sense. There are a lot of types defined in libc.

@dimbleby
Copy link
Contributor

dimbleby commented Jan 24, 2017

Just as a data point - possibly not a terribly helpful one - I'm neutral on this.

When I run bindgen in my project, I have exactly 6 types that I want bindgen to ignore, so that I can import them. Few enough that it's no big deal; though the non-recursive whitelisting would also work just fine.

I see that in the opening comment in this trail @jethrogb reports having rather more such types to deal with, and I can quite understand that it would be desirable to be able to do that without listing each one explicitly.

emilio added a commit to emilio/rust-bindgen that referenced this issue Jan 26, 2017
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

4 participants