Skip to content

Commit dd5bef5

Browse files
authored
Fix improper_ctypes warning (#254)
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.
1 parent 48195b5 commit dd5bef5

File tree

10 files changed

+125
-10
lines changed

10 files changed

+125
-10
lines changed

SwiftRustIntegrationTestRunner/SwiftRustIntegrationTestRunner/SharedStruct.swift

+4-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ func rust_calls_swift_struct_with_no_fields(arg: StructWithNoFields) -> StructWi
1111
arg
1212
}
1313

14-
func rust_calls_struct_repr_struct_one_primitive_field(arg: StructReprStructWithOnePrimitiveField) -> StructReprStructWithOnePrimitiveField {
14+
func rust_calls_swift_struct_repr_struct_one_primitive_field(arg: StructReprStructWithOnePrimitiveField) -> StructReprStructWithOnePrimitiveField {
1515
arg
1616
}
1717

18+
func rust_calls_swift_struct_repr_struct_one_string_field(arg: StructReprStructWithOneStringField) -> StructReprStructWithOneStringField {
19+
arg
20+
}

SwiftRustIntegrationTestRunner/SwiftRustIntegrationTestRunnerTests/SharedStructTests.swift

+10
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@ class SharedStructTests: XCTestCase {
3434
XCTAssertEqual(val.named_field, 56)
3535
}
3636

37+
/// Verify that we can pass a transparent struct that contains a String back and forth between Rust and Swift.
38+
/// `SharedStruct { field: String }`.
39+
func testStructReprStructWithOneStringField() {
40+
let val = rust_calls_swift_struct_repr_struct_one_string_field(
41+
arg: StructReprStructWithOneStringField(field: "hello world".intoRustString())
42+
);
43+
XCTAssertEqual(val.field.toString(), "hello world")
44+
}
45+
46+
3747
/// Verify that we can create a tuple struct.
3848
func testTupleStruct() {
3949
let val = StructReprStructTupleStruct(_0: 11, _1: 22)

crates/swift-bridge-ir/src/codegen/codegen_tests/function_attribute_codegen_tests.rs

+2
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,8 @@ mod function_attribute_swift_name_extern_rust {
429429
pub fn call_swift_from_rust() -> String {
430430
unsafe { Box::from_raw(unsafe {__swift_bridge__call_swift_from_rust () }).0 }
431431
}
432+
433+
#[allow(improper_ctypes)]
432434
extern "C" {
433435
#[link_name = "__swift_bridge__$call_swift_from_rust"]
434436
fn __swift_bridge__call_swift_from_rust() -> * mut swift_bridge::string::RustString;

crates/swift-bridge-ir/src/codegen/codegen_tests/opaque_rust_type_codegen_tests.rs

+1
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ mod extern_swift_freestanding_fn_with_owned_opaque_rust_type_arg {
422422
})) as *mut super::MyType ) }
423423
}
424424

425+
#[allow(improper_ctypes)]
425426
extern "C" {
426427
#[link_name = "__swift_bridge__$some_function"]
427428
fn __swift_bridge__some_function (arg: *mut super::MyType);

crates/swift-bridge-ir/src/codegen/codegen_tests/opaque_swift_type_codegen_tests.rs

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ mod extern_swift_freestanding_fn_with_owned_opaque_swift_type_arg {
3232
}
3333
}
3434

35+
#[allow(improper_ctypes)]
3536
extern "C" {
3637
#[link_name = "__swift_bridge__$some_function"]
3738
fn __swift_bridge__some_function (arg: MyType);

crates/swift-bridge-ir/src/codegen/codegen_tests/vec_codegen_tests.rs

+4
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,8 @@ mod extern_swift_fn_return_vec_of_primitive_rust_type {
566566
pub fn some_function() -> Vec<u8> {
567567
unsafe { *Box::from_raw(unsafe { __swift_bridge__some_function() }) }
568568
}
569+
570+
#[allow(improper_ctypes)]
569571
extern "C" {
570572
#[link_name = "__swift_bridge__$some_function"]
571573
fn __swift_bridge__some_function() -> *mut Vec<u8>;
@@ -623,6 +625,8 @@ mod extern_swift_fn_arg_vec_of_primitive_rust_type {
623625
pub fn some_function(arg: Vec<u8>) {
624626
unsafe { __swift_bridge__some_function(Box::into_raw(Box::new(arg))) }
625627
}
628+
629+
#[allow(improper_ctypes)]
626630
extern "C" {
627631
#[link_name = "__swift_bridge__$some_function"]
628632
fn __swift_bridge__some_function(arg: *mut Vec<u8>);

crates/swift-bridge-ir/src/codegen/generate_rust_tokens.rs

+85-5
Original file line numberDiff line numberDiff line change
@@ -259,11 +259,7 @@ impl ToTokens for SwiftBridgeModule {
259259
}
260260

261261
let extern_swift_fn_tokens = if extern_swift_fn_tokens.len() > 0 {
262-
quote! {
263-
extern "C" {
264-
#(#extern_swift_fn_tokens)*
265-
}
266-
}
262+
generate_extern_c_block(extern_swift_fn_tokens)
267263
} else {
268264
quote! {}
269265
};
@@ -310,6 +306,83 @@ impl ToTokens for SwiftBridgeModule {
310306
}
311307
}
312308

309+
/// Generate an `extern "C"` block such as:
310+
///
311+
/// ```no_run
312+
/// extern "C" {
313+
/// #[link_name = "some_swift_function_name"]
314+
/// fn __swift_bridge__some_swift_function_name();
315+
/// }
316+
/// ```
317+
///
318+
/// ## `improper_ctypes` lint suppression
319+
///
320+
/// We suppress the `improper_ctypes` lint with `#[allow(improper_ctypes)]`.
321+
///
322+
/// Given the following bridge module:
323+
///
324+
/// ```ignore
325+
/// #[swift_bridge::bridge]
326+
/// mod ffi {
327+
/// struct SomeStruct {
328+
/// string: String
329+
/// }
330+
///
331+
/// extern "Swift" {
332+
/// fn return_struct() -> SomeStruct;
333+
/// }
334+
/// }
335+
/// ```
336+
///
337+
/// We would generate the following struct FFI representation and `extern "C"` block:
338+
///
339+
/// ```no_run
340+
/// struct __swift_bridge__SomeStruct {
341+
/// string: *mut swift_bridge::string::RustString
342+
/// }
343+
///
344+
/// extern "C" {
345+
/// #[link_name = "__swift_bridge__$rust_calls_swift_struct_repr_struct_one_string_field"]
346+
/// fn __swift_bridge__return_struct() -> __swift_bridge__SomeStruct;
347+
/// }
348+
///
349+
/// # mod swift_bridge { pub mod string { pub struct RustString; }}
350+
/// ```
351+
///
352+
/// The `__swift_bridge__SomeStruct` holds a pointer to a `RustString`.
353+
///
354+
/// Since `RustString` is not FFI safe, and the Rust compiler cannot know that we only plan to use
355+
/// the pointer as an opaque pointer, the Rust compiler emits an `improper_ctypes` lint.
356+
///
357+
/// We silence this lint since we know that our usage is FFI safe.
358+
///
359+
/// The risk in this is that if in the future we accidentally pass a truly improper ctype over FFI
360+
/// this `#[allow(improper_ctypes)]` might prevent us from noticing.
361+
///
362+
/// Given that our codegen is heavily tested we are not currently concerned about this.
363+
///
364+
/// Should we become concerned about this in the future we could consider solutions such as:
365+
///
366+
/// - Generate an `__swift_bridge__SomeStruct_INNER_OPAQUE` that only held opaque pointers.
367+
/// We could then transmute the `__swift_bridge__SomeStruct` to/from this type when
368+
/// passing/receiving it across the FFI boundary.
369+
/// ```
370+
/// struct __swift_bridge__SomeStruct_INNER_OPAQUE {
371+
/// string: *mut std::ffi::c_void
372+
/// }
373+
/// ```
374+
/// - This would involve generating an extra type, but given that they would have the same layout
375+
/// and simply get transmuted into each other we could imagine that the optimizer would erase
376+
/// all overhead.
377+
fn generate_extern_c_block(extern_swift_fn_tokens: Vec<TokenStream>) -> TokenStream {
378+
quote! {
379+
#[allow(improper_ctypes)]
380+
extern "C" {
381+
#(#extern_swift_fn_tokens)*
382+
}
383+
}
384+
}
385+
313386
#[cfg(test)]
314387
mod tests {
315388
//! More tests can be found in src/codegen/codegen_tests.rs and its submodules.
@@ -361,6 +434,7 @@ mod tests {
361434
unsafe { __swift_bridge__some_function() }
362435
}
363436

437+
#[allow(improper_ctypes)]
364438
extern "C" {
365439
#[link_name = "__swift_bridge__$some_function"]
366440
fn __swift_bridge__some_function ();
@@ -389,6 +463,7 @@ mod tests {
389463
unsafe { __swift_bridge__some_function(start) }
390464
}
391465

466+
#[allow(improper_ctypes)]
392467
extern "C" {
393468
#[link_name = "__swift_bridge__$some_function"]
394469
fn __swift_bridge__some_function (start: bool);
@@ -620,6 +695,7 @@ mod tests {
620695
}
621696
}
622697

698+
#[allow(improper_ctypes)]
623699
extern "C" {
624700
#[link_name = "__swift_bridge__$Foo$_free"]
625701
fn __swift_bridge__Foo__free (this: *mut std::ffi::c_void);
@@ -685,6 +761,7 @@ mod tests {
685761
}
686762
}
687763

764+
#[allow(improper_ctypes)]
688765
extern "C" {
689766
#[link_name = "__swift_bridge__$Foo$new"]
690767
fn __swift_bridge__Foo_new() -> Foo;
@@ -805,6 +882,7 @@ mod tests {
805882
}
806883
}
807884

885+
#[allow(improper_ctypes)]
808886
extern "C" {
809887
#[link_name = "__swift_bridge__$Foo$notify"]
810888
fn __swift_bridge__Foo_notify(this: swift_bridge::PointerToSwiftType);
@@ -885,6 +963,7 @@ mod tests {
885963
unsafe { __swift_bridge__built_in_pointers(arg1, arg2) }
886964
}
887965

966+
#[allow(improper_ctypes)]
888967
extern "C" {
889968
#[link_name = "__swift_bridge__$built_in_pointers"]
890969
fn __swift_bridge__built_in_pointers (
@@ -912,6 +991,7 @@ mod tests {
912991
unsafe { __swift_bridge__void_pointers(arg1, arg2) }
913992
}
914993

994+
#[allow(improper_ctypes)]
915995
extern "C" {
916996
#[link_name = "__swift_bridge__$void_pointers"]
917997
fn __swift_bridge__void_pointers (

crates/swift-bridge-macro/tests/ui/incorrect-return-type.stderr

+4
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,7 @@ error[E0308]: mismatched types
2929
= note: expected struct `SomeType`
3030
found enum `Option<SomeType>`
3131
= note: this error originates in the attribute macro `swift_bridge::bridge` (in Nightly builds, run with -Z macro-backtrace for more info)
32+
help: consider using `Option::expect` to unwrap the `Option<SomeType>` value, panicking if the value is an `Option::None`
33+
|
34+
6 | #[swift_bridge::bridge].expect("REASON")
35+
| +++++++++++++++++

crates/swift-integration-tests/src/shared_types/shared_struct.rs

+13-4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ mod ffi {
1717
#[swift_bridge(swift_repr = "struct")]
1818
struct StructReprStructTupleStruct(u8, u32);
1919

20+
#[swift_bridge(swift_repr = "struct")]
21+
struct StructReprStructWithOneStringField {
22+
field: String,
23+
}
24+
2025
extern "Rust" {
2126
fn test_rust_calls_swift();
2227

@@ -34,15 +39,19 @@ mod ffi {
3439
extern "Swift" {
3540
fn rust_calls_swift_struct_with_no_fields(arg: StructWithNoFields) -> StructWithNoFields;
3641

37-
fn rust_calls_struct_repr_struct_one_primitive_field(
42+
fn rust_calls_swift_struct_repr_struct_one_primitive_field(
3843
arg: StructReprStructWithOnePrimitiveField,
3944
) -> StructReprStructWithOnePrimitiveField;
45+
46+
fn rust_calls_swift_struct_repr_struct_one_string_field(
47+
arg: StructReprStructWithOneStringField,
48+
) -> StructReprStructWithOneStringField;
4049
}
4150
}
4251

4352
fn test_rust_calls_swift() {
4453
self::tests::test_rust_calls_swift_struct_with_no_fields();
45-
self::tests::test_rust_calls_struct_repr_struct_one_primitive_field();
54+
self::tests::test_rust_calls_swift_struct_repr_struct_one_primitive_field();
4655
}
4756

4857
fn swift_calls_rust_struct_with_no_fields(arg: ffi::StructWithNoFields) -> ffi::StructWithNoFields {
@@ -70,10 +79,10 @@ mod tests {
7079
ffi::rust_calls_swift_struct_with_no_fields(ffi::StructWithNoFields);
7180
}
7281

73-
pub(super) fn test_rust_calls_struct_repr_struct_one_primitive_field() {
82+
pub(super) fn test_rust_calls_swift_struct_repr_struct_one_primitive_field() {
7483
let arg = ffi::StructReprStructWithOnePrimitiveField { named_field: 10 };
7584

76-
let val = ffi::rust_calls_struct_repr_struct_one_primitive_field(arg);
85+
let val = ffi::rust_calls_swift_struct_repr_struct_one_primitive_field(arg);
7786

7887
assert_eq!(val.named_field, 10);
7988
}

examples/async-functions/build.sh

+1
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,5 @@ cargo build -p async-functions
1010
swiftc -L ../../target/debug \
1111
-lasync_functions \
1212
-import-objc-header bridging-header.h \
13+
-framework CoreFoundation -framework SystemConfiguration \
1314
main.swift ./generated/SwiftBridgeCore.swift ./generated/async-functions/async-functions.swift

0 commit comments

Comments
 (0)