Skip to content

Commit

Permalink
Suppress dead_code warning when returning `-> Result<*, TransparentTy…
Browse files Browse the repository at this point in the history
…pe>` (#278)

In Rust 1.79.0, dead code warnings are emitted from enums where the compiler does not infer that wrapped data is used, even when the enum has been annotated with `#[repr(C]`. This warning triggers in `swift-bridge` when defining transparent structs or enums that will be returned in error results.

This appears to be a regression in the `dead_code` rustc lint. Pending upstream fixes to rustc, this change to `swift-bridge` annotates generated result enums with `#[allow(unused)]`.

Fixes #270

```
error: field `0` is never read
   |
1  | #[swift_bridge::bridge]
   | ----------------------- field in this variant
...
3  |     enum ResultTransparentEnum {
   |          ^^^^^^^^^^^^^^^^^^^^^
   |
help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field
   |
3  |     enum () {
   |          ~~
```

## Example bridge

The following bridge code is the minimal reproduction case:

``` rust
#[swift_bridge::bridge]
mod ffi {
    #[swift_bridge(swift_repr = "struct")]
    struct TransparentErrorStruct(pub String);

    extern "Rust" {
        fn rust_func_returns_result_transparent_struct(
            succeed: bool
        ) -> Result<(), TransparentErrorStruct>;
    }
}

fn rust_func_returns_result_transparent_struct(
    succeed: bool
) -> Result<(), ffi::ResultTestTransparentStruct> {
    if succeed {
        Ok(())
    } else {
        Err(ffi::ResultTestTransparentStruct("failed".to_string()))
    }
}

impl std::error::Error for ffi:: TransparentErrorStruct {}

impl std::fmt::Debug for ffi:: TransparentErrorStruct {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "{}", self)
    }
}

impl std::fmt::Display for ffi:: TransparentErrorStruct {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "{}", self.0)
    }
}
```
  • Loading branch information
sax authored Jun 21, 2024
1 parent 717fcef commit 34ce1ff
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ extension AsyncResultOpaqueRustType2: Error {}
extension ResultTransparentEnum: @unchecked Sendable {}
extension ResultTransparentEnum: Error {}

extension ResultTransparentStruct: @unchecked Sendable {}
extension ResultTransparentStruct: Error {}

extension SameEnum: @unchecked Sendable {}
extension SameEnum: Error {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,46 @@ class ResultTests: XCTestCase {
}
}

/// Verify that we can receive a Result<(), TransparentStruct> from Rust
func testResultNullTransparentStruct() throws {
try! rust_func_return_result_null_transparent_struct(true)

do {
try rust_func_return_result_null_transparent_struct(false)
XCTFail("The function should have returned an error.")
} catch let error as ResultTransparentStruct {
XCTAssertEqual(error.inner.toString(), "failed")
}

XCTContext.runActivity(named: "Should return a Unit type") {
_ in
do {
let _ :() = try rust_func_return_result_unit_type_enum_opaque_rust(true)
} catch {
XCTFail()
}
}

XCTContext.runActivity(named: "Should throw an error") {
_ in
do {
let _ :() = try rust_func_return_result_unit_type_enum_opaque_rust(false)
XCTFail("The function should have returned an error.")
} catch let error as ResultTransparentEnum {
switch error {
case .NamedField(let data):
XCTAssertEqual(data, 123)
case .UnnamedFields(_, _):
XCTFail()
case .NoFields:
XCTFail()
}
} catch {
XCTFail()
}
}
}

/// Verify that we can receive a Result<Vec<>, OpaqueRust> from Rust
func testSwiftCallRustResultVecUInt32Rust() throws {
let vec = try! rust_func_return_result_of_vec_u32()
Expand Down
4 changes: 4 additions & 0 deletions crates/swift-bridge-ir/src/bridged_type/bridgeable_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,14 @@ impl BuiltInResult {
.err_ty
.to_ffi_compatible_rust_type(swift_bridge_path, types);
let mut custom_rust_ffi_types = vec![];
// TODO: remove `#[allow(unused)]` when rustc no longer issues dead code warnings for `#[repr(C)]`
// structs or enums: https://github.com/rust-lang/rust/issues/126706
custom_rust_ffi_types.push(quote! {
#[repr(C)]
pub enum #ty {
#[allow(unused)]
Ok #ok,
#[allow(unused)]
Err(#err),
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,11 +407,16 @@ mod extern_rust_fn_return_result_opaque_rust_type_and_transparent_enum_type {
}
}

// In Rust 1.79.0 dead_code warnings are issued for wrapped data in enums in spite of the enum
// having `#[repr(C)]`. `#[allow(unused)]` can be removed following resolution and release of this
// issue: https://github.com/rust-lang/rust/issues/126706
fn expected_rust_tokens() -> ExpectedRustTokens {
ExpectedRustTokens::Contains(quote! {
#[repr(C)]
pub enum ResultSomeOkTypeAndSomeErrEnum{
#[allow(unused)]
Ok(*mut super::SomeOkType),
#[allow(unused)]
Err(__swift_bridge__SomeErrEnum),
}

Expand Down Expand Up @@ -484,11 +489,16 @@ mod extern_rust_fn_return_result_transparent_enum_type_and_opaque_rust_type {
}
}

// In Rust 1.79.0 dead_code warnings are issued for wrapped data in enums in spite of the enum
// having `#[repr(C)]`. `#[allow(unused)]` can be removed following resolution and release of this
// issue: https://github.com/rust-lang/rust/issues/126706
fn expected_rust_tokens() -> ExpectedRustTokens {
ExpectedRustTokens::Contains(quote! {
#[repr(C)]
pub enum ResultSomeOkEnumAndSomeErrType{
#[allow(unused)]
Ok(__swift_bridge__SomeOkEnum),
#[allow(unused)]
Err(*mut super::SomeErrType),
}

Expand Down Expand Up @@ -558,11 +568,16 @@ mod extern_rust_fn_return_result_unit_type_and_transparent_enum_type {
}
}

// In Rust 1.79.0 dead_code warnings are issued for wrapped data in enums in spite of the enum
// having `#[repr(C)]`. `#[allow(unused)]` can be removed following resolution and release of this
// issue: https://github.com/rust-lang/rust/issues/126706
fn expected_rust_tokens() -> ExpectedRustTokens {
ExpectedRustTokens::Contains(quote! {
#[repr(C)]
pub enum ResultVoidAndSomeErrEnum{
#[allow(unused)]
Ok,
#[allow(unused)]
Err(__swift_bridge__SomeErrEnum),
}

Expand Down Expand Up @@ -628,12 +643,17 @@ mod extern_rust_fn_return_result_tuple_type_and_transparent_enum_type {
}
}

// In Rust 1.79.0 dead_code warnings are issued for wrapped data in enums in spite of the enum
// having `#[repr(C)]`. `#[allow(unused)]` can be removed following resolution and release of this
// issue: https://github.com/rust-lang/rust/issues/126706
fn expected_rust_tokens() -> ExpectedRustTokens {
ExpectedRustTokens::ContainsMany(vec![
quote! {
#[repr(C)]
pub enum ResultTupleI32U32AndSomeErrEnum{
#[allow(unused)]
Ok(__swift_bridge__tuple_I32U32),
#[allow(unused)]
Err(__swift_bridge__SomeErrEnum),
}
},
Expand Down Expand Up @@ -687,3 +707,77 @@ typedef struct __swift_bridge__$ResultTupleI32U32AndSomeErrEnum{__swift_bridge__
.test();
}
}

/// Test code generation for Rust function that returns a Result<T, E> where T is () and
/// E is a transparent struct type.
mod extern_rust_fn_return_result_unit_type_and_transparent_struct_type {
use super::*;

fn bridge_module_tokens() -> TokenStream {
quote! {
mod ffi {
struct SomeErrStruct {
inner: String,
}
extern "Rust" {
fn some_function() -> Result<(), SomeErrStruct>;
}
}
}
}

// In Rust 1.79.0 dead_code warnings are issued for wrapped data in enums in spite of the enum
// having `#[repr(C)]`. `#[allow(unused)]` can be removed following resolution and release of this
// issue: https://github.com/rust-lang/rust/issues/126706
fn expected_rust_tokens() -> ExpectedRustTokens {
ExpectedRustTokens::Contains(quote! {
#[repr(C)]
pub enum ResultVoidAndSomeErrStruct{
#[allow(unused)]
Ok,
#[allow(unused)]
Err(__swift_bridge__SomeErrStruct),
}

#[export_name = "__swift_bridge__$some_function"]
pub extern "C" fn __swift_bridge__some_function() -> ResultVoidAndSomeErrStruct{
match super::some_function() {
Ok(ok) => ResultVoidAndSomeErrStruct::Ok,
Err(err) => ResultVoidAndSomeErrStruct::Err(err.into_ffi_repr()),
}
}
})
}

fn expected_swift_code() -> ExpectedSwiftCode {
ExpectedSwiftCode::ContainsAfterTrim(
r#"
public func some_function() throws -> () {
try { let val = __swift_bridge__$some_function(); switch val.tag { case __swift_bridge__$ResultVoidAndSomeErrStruct$ResultOk: return case __swift_bridge__$ResultVoidAndSomeErrStruct$ResultErr: throw val.payload.err.intoSwiftRepr() default: fatalError() } }()
}
"#,
)
}

fn expected_c_header() -> ExpectedCHeader {
ExpectedCHeader::ContainsManyAfterTrim(vec![
r#"
typedef enum __swift_bridge__$ResultVoidAndSomeErrStruct$Tag {__swift_bridge__$ResultVoidAndSomeErrStruct$ResultOk, __swift_bridge__$ResultVoidAndSomeErrStruct$ResultErr} __swift_bridge__$ResultVoidAndSomeErrStruct$Tag;
union __swift_bridge__$ResultVoidAndSomeErrStruct$Fields {struct __swift_bridge__$SomeErrStruct err;};
typedef struct __swift_bridge__$ResultVoidAndSomeErrStruct{__swift_bridge__$ResultVoidAndSomeErrStruct$Tag tag; union __swift_bridge__$ResultVoidAndSomeErrStruct$Fields payload;} __swift_bridge__$ResultVoidAndSomeErrStruct;
"#,
r#"struct __swift_bridge__$ResultVoidAndSomeErrStruct __swift_bridge__$some_function(void)"#,
])
}

#[test]
fn extern_rust_result_transparent_struct_type_and_opaque_rust_type() {
CodegenTest {
bridge_module: bridge_module_tokens().into(),
expected_rust_tokens: expected_rust_tokens(),
expected_swift_code: expected_swift_code(),
expected_c_header: expected_c_header(),
}
.test();
}
}
41 changes: 37 additions & 4 deletions crates/swift-integration-tests/src/result.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
//! See also: crates/swift-bridge-ir/src/codegen/codegen_tests/result_codegen_tests.rs
// This is a temporary workaround until https://github.com/chinedufn/swift-bridge/issues/270
// is closed. When tests are compiled they have `-D warnings` (deny warnings) enabled, so
// tests won't even compile unless this warning is ignored.
#![allow(dead_code)]
#[swift_bridge::bridge]
mod ffi {
Expand Down Expand Up @@ -41,6 +37,17 @@ mod ffi {
fn val(&self) -> u32;
}

#[swift_bridge(swift_repr = "struct")]
struct ResultTransparentStruct {
pub inner: String,
}

extern "Rust" {
fn rust_func_return_result_null_transparent_struct(
succeed: bool,
) -> Result<(), ResultTransparentStruct>;
}

enum ResultTransparentEnum {
NamedField { data: i32 },
UnnamedFields(u8, String),
Expand Down Expand Up @@ -141,6 +148,32 @@ fn rust_func_return_result_unit_struct_opaque_rust(
}
}

fn rust_func_return_result_null_transparent_struct(
succeed: bool,
) -> Result<(), ffi::ResultTransparentStruct> {
if succeed {
Ok(())
} else {
Err(ffi::ResultTransparentStruct {
inner: "failed".to_string(),
})
}
}

impl std::error::Error for ffi::ResultTransparentStruct {}

impl std::fmt::Debug for ffi::ResultTransparentStruct {
fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
unreachable!("Debug impl was added to pass `Error: Debug + Display` type checking")
}
}

impl std::fmt::Display for ffi::ResultTransparentStruct {
fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
unreachable!("Display impl was added to pass `Error: Debug + Display` type checking")
}
}

pub struct ResultTestOpaqueRustType {
val: u32,
}
Expand Down

0 comments on commit 34ce1ff

Please sign in to comment.