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

improper_ctypes lint fires on #[repr(transparent)] wrappers around PhantomData (in struct fields). #106629

Closed
thomcc opened this issue Jan 9, 2023 · 2 comments · Fixed by #106675
Assignees
Labels
A-FFI Area: Foreign function interface (FFI) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@thomcc
Copy link
Member

thomcc commented Jan 9, 2023

If I have a #[repr(transparent)] wrapper around a PhantomData, it's considered an improper C type when used in a struct field, even though the wrapped PhantomData would be allowed. Specifically (playground)

#[repr(transparent)]
#[derive(Copy, Clone)]
struct MyPhantom(core::marker::PhantomData<u8>);

#[repr(C)]
#[derive(Copy, Clone)]
pub struct Bar {
    pub x: i32,
    _marker: MyPhantom,
}

extern "C" {
    pub fn foo(bar: *mut Bar);
}

produces the error

   Compiling playground v0.0.1 (/playground)
warning: `extern` block uses type `MyPhantom`, which is not FFI-safe
  --> src/lib.rs:13:21
   |
13 |     pub fn foo(bar: *mut Bar);
   |                     ^^^^^^^^ not FFI-safe
   |
   = note: this struct contains only zero-sized fields
note: the type is defined here
  --> src/lib.rs:3:1
   |
3  | struct MyPhantom(core::marker::PhantomData<u8>);
   | ^^^^^^^^^^^^^^^^
   = note: `#[warn(improper_ctypes)]` on by default

warning: `playground` (lib) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.51s

This is over-zealous for a few reasons.

  1. PhantomData on its own is allowed in C structs, so changing this to _marker: PhantomData<u8> would solve the warning (but defeats the point of my use case1).

  2. #[repr(transparent)] is supposed to be treated as the inner type for purposes of ABI, and the improper_ctypes lint is about ABI.

  3. Using #[repr(C)] instead of #[repr(transparent)] solves this technically (it shuts the warning up), but seems pretty dodgy, since it's not actually laid out the way C would lay it out anymore (that is, we've guaranteed that PhantomData is never going to impact layout at all, and we haven't guaranteed this for zero-sized #[repr(C)] types which are notably not a real thing in C).

TLDR: this lint fires on #[repr(transparent)] wrappers around PhantomData when they're used as struct fields. IMO these should be fine since PhantomData doesn't emit a warning in that case, and #[repr(transparent)] is supposed to defer to the inner type.

(That said, firing this lint on a type passed by pointer is pretty weird anyway, since the indirection means doesn't actually have to be a valid C type (for the ABI), which is the basic complaint behind #66373, which is not what I'm complaining about. IMO this shouldn't fire even if Bar were being passed by value, as making the field PhantomData (instead of a wrapper around one) doesn't fire in that case)

Footnotes

  1. If you're curious about the real use case I have, a closer but still-basically-minimized version is here. It's important that the type not be a direct PhantomData (basically #[non_exhaustive] is not really usable for FFI, but C has cases where considers appending new fields to the end of a type to be a non-breaking change).

@thomcc thomcc added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-FFI Area: Foreign function interface (FFI) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jan 9, 2023
@krtab
Copy link
Contributor

krtab commented Jan 9, 2023

I'd like to take a look at this.
@rustbot claim

@krtab
Copy link
Contributor

krtab commented Jan 9, 2023

Related:
#94000
#66373

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants