PhantomData<T> doesn't trigger improper_ctypes_definitions lint

[PonasKovas]

Given the following code:

use std::marker::PhantomData;

#[repr(C)]
struct Ptr<T> {
    ptr: *const T,
    _phantom: PhantomData<T>,
}

extern "C" fn test(ptr: Ptr<String>) {}

Based on the documentation of PhantomData:

Adding a PhantomData field to your type tells the compiler that your type acts as though it stores a value of type T, even though it doesn’t really.

I would think that it should trigger the improper_ctypes_definitions compiler lint, like this example would:

#[repr(C)]
struct Ptr<T> {
    inner: T
}

extern "C" fn test(ptr: Ptr<String>) {}

warning: `extern` fn uses type `String`, which is not FFI-safe
 --> src/lib.rs:6:25
  |
6 | extern "C" fn test(ptr: Ptr<String>) {}
  |                         ^^^^^^^^^^^ not FFI-safe
  |
  = note: `#[warn(improper_ctypes_definitions)]` on by default
  = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
  = note: this struct has unspecified layout

I think that it is intuitive that PhantomData<T> should be marked as not FFI-safe if T is not FFI-safe.

There is a workaround to this by using [T; 0] instead of PhantomData<T>, but it is very hacky:

#[repr(C, packed)]
struct PhantomType<T>([T; 0]);

#[repr(C)]
struct Ptr<T> {
    ptr: *const T,
    _phantom: PhantomType<T>,
}

extern "C" fn test(ptr: Ptr<String>) {} // gives warning

[wwylele]

Should Ptr not be FFI-safe? The definition of it is basically the definition of std::boxed::Box, yet Box is FFI-safe.

[PonasKovas]

If your `Ptr` has methods which assume it is FFI-safe (like dereferencing the pointer and doing things with it), then you would want to make sure that `T` is FFI-safe, even though the type signature doesn't require it. That's when you would want to add `PhantomData<T>`.

[wwylele]

It sounds to me that we need a different marker like PhantomFFIUnsafeData for this purpose. or just make it not #[repr(C)]. PhantomData<T> is not used for marking FFI safety. I believe that for this piece of doc

your type acts as though it stores a value of type T

it means that the type stores a T in terms of lifetime and ownership (which borrow checker etc cares, but ABI doesn't care), but not in terms of data layout (which borrow checker doesn't care, but ABI does). Maybe we should clarify this in the doc.

[PonasKovas]

PhantomData<T> is not used for marking FFI safety.

Is there a reason why?

or just make it not #[repr(C)]

I don't think you understand my problem. Here's a more specific example: say you wanted to make a FFI-safe Vec. Since Vec can be easily disassembled to it's raw parts (pointer, length and capacity), you can create

#[repr(C)]
pub struct SafeVec<T> {
    ptr: *mut T,
    length: usize,
    capacity: usize
}

And implement methods for converting from/to the original Vec.

Now the problem is that you can use any T in this SafeVec<T> and it will never fire the improper_ctypes_definitions warning. In theory it is correct, because it's just a pointer to T and has a stable and well-defined ABI. But if you really want to use SafeVec as a drop-in replacement of Vec for FFI stuff, you would still want to get the warning if you accidentally put something that is not FFI-safe in there. So naturally the first idea is to add a field _phantom: PhantomData<T>, so the compiler would think that SafeVec has T, which is not FFI-safe and would give the warning. That is not the case, though, and that's what this issue is about.

As I said before, there's a workaround to use a zero-sized array of that type [T; 0], but it has it's own drawbacks too. For example, if you use it, you can only implement Copy for your structure if T: Copy, even though there isn't actually T in the struct.

[thomcc]

This suggestion would add a ton of new false positives to the improper_ctypes lints, and go against common idioms like using the closest type possible (for example, using a PhantomData<&'a [u8]> in an std::io::IoSlice<'a> (just one example of many in the stdlib), or perhaps a PhantomData<Vec<T>> for your FfiSafeVec<T>, depending on how your type was written).

In general we try to make the builtin lints have a low degree of false positives, especially the ones which are enabled by default. Sadly, the improper_ctypes lints are already significant outliers here, and fire often enough to cause real problems (especially given that they fire at the site where the type is used rather than where it is defined, which means they can't be #[allow]ed by the author of a given type). I don't think we should make that any worse.

Also keep in mind that PhantomData is often required to get correct variance and dropck behavior, as well as to to erase type and lifetime parameters, and so on. It's often not optional to add it, so it would be quite frustrating to have a reason the most accurate PhantomData couldn't be used (since it can be pretty difficult to come up with an equivalent that doesn't have some subtle difference in terms of variance, auto traits, dropck, etc).

[PonasKovas]

I now understand that my original proposal is not good and I agree with your points.

Is there any way my problem with SafeVec<T> could be solved? Maybe an attribute macro that would let users "mark" their types as FFI-safe or not based on generics, not sure how though, or if it's even possible, just an idea?

[workingjubilee]

Closing this as non-actionable due to the reasons that Thom cited.