PhantomData is not foreign-function safe

[rillian]

On my system (Fedora Linux 25) bindgen generates a PhantomData-based union representation for fpos_t from stdio. Rustc (1.14 or 1.15) complains about this:

warning: found non-foreign-function-safe member in struct marked #[repr(C)]: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type, #[warn(improper_ctypes)] on by default
14 |     pub fn fgetpos(__stream: *mut FILE, __pos: *mut fpos_t)
   |                                                ^^^^^^^^^^^

Here's a reduced example:

#[repr(C)]
pub struct FILE;

#[repr(C)]
#[allow(non_camel_case_types)]
pub struct fpos_t {
    pub __value: ::std::marker::PhantomData<::std::os::raw::c_uint>,
}

extern "C" {
    pub fn fgetpos(__stream: *mut FILE, __pos: *mut fpos_t)
     -> ::std::os::raw::c_int;
}

It would be good if the generated code compiled without warnings.

[emilio]

That is (unfortunately) known, see rust-lang/rust#34798. This is a lint problem though.

This goes away in nightly rust if you use native unions, of course, but until that's fixed... We could only alter our union representation to use setters/getters instead of fields, but I implemented the current solution because people complained that getters/setters were too annoying :/

[emilio]

Also, we cannot workaround these warnings for things like:

template <typename T>
struct Foo {
    int bar;
};

[rillian]

Thanks for the follow up. I'm still confused about whether the lint is a real issue or not. If PhantomData is just markup for the borrow checker, C shouldn't care that that's zero-sized, and the lint would be incorrect. Or is it actually a safety leak?

So maybe the lint should except this type? Or we could stabilize an #[allow(non_foreign_function_safe)] annotation if we know our codegen is safe? How can we resolve this?

[emilio]

Thanks for the follow up. I'm still confused about whether the lint is a real issue or not. If PhantomData is just markup for the borrow checker, C shouldn't care that that's zero-sized, and the lint would be incorrect. Or is it actually a safety leak?

It's a lint issue as far as I understand it.

So maybe the lint should except this type? Or we could stabilize an #[allow(non_foreign_function_safe)] annotation if we know our codegen is safe? How can we resolve this?

Yeah, an exception for the lint is the best thing (I never looked into implementing it, but doesn't sound too difficult). Alternatively, #[allow(improper_ctypes)] has been stable for quite a long time :)

[emilio]

I'll try to whitelist PhantomData, and see how it turns like.

[barafael]

I am having this exact issue. Are the structs I am passing to C still represented correctly? The C library I am working with behaves differently when I call call it with Rust and C. Could there be a representation issue?

[emilio]

Yes, we generate assertions so that the types and offsets are correct. With template parameters it's a bit harder, but it's not a problem of PhantomData per se.

BTW, I landed a PR in rust to avoid the warning.

I'm happy to take a look at the code if you hand me a link to see what can be going wrong :)

[barafael]

PR: awesome ;)

My code is at:
https://github.com/medium-endian/crisp/tree/master/rusp

Library works fine when using C (same repo) https://github.com/medium-endian/crisp/blob/master/cisp/parsing.c

parsing.c and main.rs are supposed do the same thing :)

[emilio]

Your code acts wrong because you made a typo, but the bindings work correctly.

This code:

if (mpc_parse(stdin_str.as_ptr(), input.as_ptr(), number, &mut r)) != 0 {

Should read:

if (mpc_parse(stdin_str.as_ptr(), input.as_ptr(), lispy, &mut r)) != 0 {

With that both do the same :)

[emilio]

Also, probably you want to know that you can use b"<stdin>\0".as_ptr() as *const _ instead of calling CString::new("<stdin>") all the time :)

[barafael]

Oh my, thank you so much. Really can't thank enough. Sorry for accusing the bindings!

[emilio]

This should be fixed now: rust-lang/rust#39462