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

[Feature]: Make FlagSet struct FFI-safe #27

Closed
1 task done
Kiuhbit opened this issue Apr 15, 2023 · 0 comments
Closed
1 task done

[Feature]: Make FlagSet struct FFI-safe #27

Kiuhbit opened this issue Apr 15, 2023 · 0 comments
Labels
enhancement New feature or request

Comments

@Kiuhbit
Copy link

Kiuhbit commented Apr 15, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Description

Currently, the FlagSet struct doesn't have a repr attribute on it, which means it's technically not FFI-safe.

When writing an extern fn which takes a FlagSet or a struct with a FlagSet field, I get the following compiler warning:

warning: `extern` fn uses type `FlagSet<ExampleFlags>`, which is not FFI-safe
  --> src/lib.rs:16:43
   |
16 | pub extern fn do_thing_with_flags(_flags: FlagSet<ExampleFlags>) {
   |                                           ^^^^^^^^^^^^^^^^^^^^^ 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_definitions)]` on by default

Here's the complete example code I used to generate that warning:

lib.rs:

use flagset::{FlagSet, flags};

flags! {
    #[repr(C)]
    pub enum ExampleFlags: u32 {
        Flag1,
        Flag2,
    }
}

#[repr(C)]
pub struct ExampleStruct {
    pub flags: FlagSet<ExampleFlags>
}

pub extern "C" fn do_thing_with_flags(_flags: FlagSet<ExampleFlags>) {
    // code here
}

pub extern "C" fn do_things_with_struct(_example_struct: ExampleStruct) {
    // code here
}

Cargo.toml:

[package]
name = "ffitest"
version = "0.1.0"
edition = "2021"

[dependencies]
flagset = "0.4.3"

Acceptance Criteria

FlagSet structs can be used in extern functions without improper_ctypes_definitions compiler warnings.

Suggestions for a technical implementation

Add #[repr(transparent)] or #[repr(C)] to the FlagSet struct. I think this would technically be considered a breaking change, although I doubt it makes any difference to the actual memory layout of a tuple struct with a single value.

@Kiuhbit Kiuhbit added the enhancement New feature or request label Apr 15, 2023
rjzak added a commit to rjzak/flagset that referenced this issue Mar 3, 2024
Signed-off-by: Richard Zak <richard.j.zak@gmail.com>
@rjzak rjzak closed this as completed in 34afef7 Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

1 participant