Enable #[deny(improper_ctypes)]

[jschwe]

When building moz-js locally as part of servo I'm getting some warnings about some function parameters not being FFI-safe. Is this a known issue, that can't be worked around? In my opinion we should deny improper_ctypes by default, and perhaps allow certain cases if it really can't be avoided and people are extremely sure that in practice the types are ffi-safe.

[wusyong]

I think this is fine for now because we use same target build on the same platform, or even build one for you own. But if we want to get rid of the warning, I think we can just blacklist all those types. Many of these are not actually used in servo. Even we need it, we could manually write one for our own.

[jschwe]

I'm seeing an example here where an array is passed directly as a function argument: https://github.com/servo/mozjs/blob/3720f6d208b45fb968961a8d5c97f64010ae3d81/mozjs/src/rust.rs#L362

The unsafe code guidelines explicit list array arguments as not FFI safe, and say a pointer to the array needs to be used instead to match the behavior on the C-side.

[jschwe]

Here is an example on godbolt showing the different code Rust generates for pass by value vs pass by reference of an array: https://godbolt.org/z/vhqzv5fee Note that in the by value case, the values are passed via 2 registers, while in the by reference case one register is used to pass a pointer to the array.

In C/C++ arrays are passed by pointer, so the binding needs to do the same (I guess this is also an upstream bindgen issue)

Edit: https://github.com/rust-lang/rust-bindgen/issues/2071 seems to indicate we must blocklist the underlying C++ type, since bindgen is known to not work well for such types.

[sagudev]

I plan to take some time and clean up mozjs (warnings, code, pull request, issues), but this will probably happen in september.

we must blocklist the underlying C++ type, since bindgen is known to not work well for such types.

This will require more glue code.