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

Fix improper_ctypes warning #254

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Conversation

chinedufn
Copy link
Owner

@chinedufn chinedufn commented Mar 8, 2024

This commit fixes an improper_ctypes warning when bridging transparent
structs that contain non FFI safe types.

While transparent structs that contained non FFI safe types were being
passed in an FFI safe way before this commit, the Rust compiler could
not know that what we were doing was FFI safe.

This commit uses #[allow(improper_ctypes)] to silence the lint.

Illustration

Before this commit the following bridge module:

#[swift_bridge::bridge]
mod ffi {
    struct SharedStruct {
        field: String
    }

    extern "Swift" {
        fn swift_func() -> SharedStruct;
    }
}

would lead to the following warning:

warning: `extern` block uses type `RustString`, which is not FFI-safe
 --> my_file.rs:4:12
  |
4 |     struct SharedStruct {
  |            ^^^^^^^^^^^^ not FFI-safe
  |
  = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
  = note: this struct has unspecified layout
  = note: `#[warn(improper_ctypes)]` on by default

This warning was a bit misleading in that it made it seem like the
RustString needed a #[repr(C)] annotation.

The real issue was that the generated FFI representation type:

#[repr(C)]
struct __swift_bridge__SharedStruct {
    field: *mut std::string::RustString
}

was triggering a warning because the Rust compiler can't know that the
non-FFI safe std::string::RustString is not being dereferenced on the
Swift side.

@chinedufn chinedufn force-pushed the fix-improper-ctypes-warning branch 3 times, most recently from 5950081 to bcd17dc Compare March 8, 2024 02:22
@chinedufn chinedufn force-pushed the fix-improper-ctypes-warning branch from bcd17dc to edac4cf Compare March 8, 2024 02:25
This commit fixes an `improper_ctypes` warning when bridging transparent
structs that contain non FFI safe types.

While transparent structs that contained non FFI safe types were being
passed in an FFI safe way before this commit, the Rust compiler could
not know that what we were doing was FFI safe.

This commit uses `#[allow(improper_ctypes)]` to silence the lint.

## Illustration

Before this commit the following bridge module:

```rust
#[swift_bridge::bridge]
mod ffi {
    struct SharedStruct {
        field: String
    }

    extern "Swift" {
        fn swift_func() -> SharedStruct;
    }
}
```

would lead to the following warning:

```console
warning: `extern` block uses type `RustString`, which is not FFI-safe
 --> my_file.rs:4:12
  |
4 |     struct SharedStruct {
  |            ^^^^^^^^^^^^ not FFI-safe
  |
  = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
  = note: this struct has unspecified layout
  = note: `#[warn(improper_ctypes)]` on by default
```

This warning was a bit misleading in that it made it seem like the
`RustString` needed a `#[repr(C)]` annotation.

The real issue was that the generated FFI representation type:

```rust
#[repr(C)]
struct __swift_bridge__SharedStruct {
    field: *mut std::string::RustString
}
```
was triggering a warning because the Rust compiler can't know that the
non-FFI safe `std::string::RustString` is not being dereferenced on the
Swift side.
@chinedufn chinedufn force-pushed the fix-improper-ctypes-warning branch from edac4cf to fc78618 Compare March 8, 2024 02:31
@chinedufn chinedufn merged commit dd5bef5 into master Mar 8, 2024
5 checks passed
@chinedufn chinedufn deleted the fix-improper-ctypes-warning branch March 8, 2024 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant