Support returning `Result<tuple, Any Type>` from Rust functions #213

NiwakaDev · 2023-04-23T05:32:58Z

This commit adds support for returning Result<tuple, Any Type> from Rust functions. Note that this doesn't support returning Result<Any Type, tuple> from Rust functions.

Here's an example of using this.

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        fn rust_func_return_result_tuple_transparent_enum(
            succeed: bool,
        ) -> Result<(i32, ResultTestOpaqueRustType, String), ResultTransparentEnum>;
    }
}

//...
do {
    //...
    let tuple: (Int32, ResultTestOpaqueRustType, RustString) = try rust_func_return_result_tuple_transparent_enum(true)
    //...
} catch {
    //...
}
//...

NiwakaDev · 2023-04-23T05:35:16Z

crates/swift-bridge-ir/src/bridged_type/bridgeable_result.rs

-        let mut ok_and_err = trimmed.split(",");
-        let ok = ok_and_err.next()?.trim();
-        let err = ok_and_err.next()?.trim();
+        let ok_and_err = trimmed.rsplit_once(",")?;


If we want to support returning Result<Any Type, tuple> from Rust functions, we need to be able to parse Result<SomeType, (i32, u32, String)> or something like that.

NiwakaDev · 2023-04-23T05:51:12Z

@chinedufn

In my local environment(rustc version is 1.68), tests pass successfully. So, I guess that tests fail because rustc version is 1.69.

Actually, when I changed rustc version to 1.69 in my local environment, tests fail.

NiwakaDev · 2023-04-23T08:38:28Z

When running tests/ui/incorrect-return-type.rs on rustc(version 1.69), tests fail.

test tests/ui/incorrect-return-type.rs ... mismatch

EXPECTED:
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
error[E0308]: mismatched types
  --> tests/ui/incorrect-return-type.rs:6:1
   |
6  |   #[swift_bridge::bridge]
   |   ^^^^^^^^^^^^^^^^^^^^^^^ expected struct `SomeType`, found `&SomeType`
...
9  |           type SomeType;
   |  ______________-
10 | |
11 | |         #[swift_bridge(rust_name = "some_function")]
   | |_________- expected due to this
   |
   = note: this error originates in the attribute macro `swift_bridge::bridge` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
  --> tests/ui/incorrect-return-type.rs:6:1
   |
6  |   #[swift_bridge::bridge]
   |   ^^^^^^^^^^^^^^^^^^^^^^^ expected struct `SomeType`, found enum `Option`
...
9  |           type SomeType;
   |  ______________-
10 | |
11 | |         #[swift_bridge(rust_name = "some_function")]
12 | |         fn fn1() -> SomeType;
13 | |         #[swift_bridge(rust_name = "another_function")]
   | |_________- expected due to this
   |
   = note: expected struct `SomeType`
                found enum `Option<SomeType>`
   = note: this error originates in the attribute macro `swift_bridge::bridge` (in Nightly builds, run with -Z macro-backtrace for more info)
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈

ACTUAL OUTPUT:
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
error[E0308]: mismatched types
  --> tests/ui/incorrect-return-type.rs:6:1
   |
6  |   #[swift_bridge::bridge]
   |   ^^^^^^^^^^^^^^^^^^^^^^^ expected `SomeType`, found `&SomeType`
...
9  |           type SomeType;
   |  ______________-
10 | |
11 | |         #[swift_bridge(rust_name = "some_function")]
   | |_________- expected due to this
   |
   = note: this error originates in the attribute macro `swift_bridge::bridge` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
  --> tests/ui/incorrect-return-type.rs:6:1
   |
6  |   #[swift_bridge::bridge]
   |   ^^^^^^^^^^^^^^^^^^^^^^^ expected `SomeType`, found `Option<SomeType>`
...
9  |           type SomeType;
   |  ______________-
10 | |
11 | |         #[swift_bridge(rust_name = "some_function")]
12 | |         fn fn1() -> SomeType;
13 | |         #[swift_bridge(rust_name = "another_function")]
   | |_________- expected due to this
   |
   = note: expected struct `SomeType`
                found enum `Option<SomeType>`
   = note: this error originates in the attribute macro `swift_bridge::bridge` (in Nightly builds, run with -Z macro-backtrace for more info)
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈

chinedufn · 2023-04-24T13:23:33Z

Gotcha. Yeah different versions of Rust will sometimes change the output.

Want to update the UI test?

chinedufn

Minor feedback

chinedufn · 2023-04-24T13:29:00Z

crates/swift-bridge-ir/src/bridged_type/bridgeable_result.rs

+                custom_rust_ffi_types.push(custom_rust_ffi_type);
+            }
+        }
+        return Some(custom_rust_ffi_types);


What about Some(quote! { #(#custom_rust_ffi_types)* }) ?

Let's say that we have the following code:

#[swift_bridge::bridge] mod ffi { extern "Rust" { fn some_function1( ) -> Result<(i32, ResultTestOpaqueRustType, String), ResultTransparentEnum>; } extern "Rust" { fn some_function2( ) -> (i32, ResultTestOpaqueRustType, String); } }

The contents of a HashMap are as follows:

[result definition and (i32, ResultTestOpaqueRustType, String) definition and ResultTransparentEnum definition, (i32, ResultTestOpaqueRustType, String) definition]

So, the (i32, ResultTestOpaqueRustType, String) definition should generate twice.

chinedufn · 2023-04-24T13:33:35Z

crates/swift-bridge-ir/src/bridged_type.rs

        &self,
        swift_bridge_path: &Path,
        types: &TypeDeclarations,
-    ) -> Option<TokenStream>;
+    ) -> Option<Vec<TokenStream>>;


I don't think that we should change any of these to Vec<T>

Imagine the following bridge module:

#[swift_bridge::bridge] mod ffi { extern "Rust" { fn a() -> Result<(u32, u32), ()> fn b() -> Result<(u32, u32), ()> } }

Right now It think we should generate the FFI types for (u32, u32) twice, I believe (I might be wrong, I don't remember).

This bug is fine for now, but in the future we'll want to only ever generate the support types once.

Generating the support types while generating Result<T, E> support types won't work in that future since we'd be generating the support types multiple times.

Instead, in this future, we would want to scan all bridge modules up front and generate support types once per type occurrence. i.e. if we would generate the support types for the (u32, u32) only once.

We don't need to implement all of this in this PR, I'm just describing the future.

For this PR, though, I think we should remove the Vec<T> changes.

I left a note below on how you can accomplish what you want without introducing a Vec.

Right now It think we should generate the FFI types for (u32, u32) twice

Custom definitions is managed by HashMap, so it cannot be generated twice.

swift-bridge/crates/swift-bridge-ir/src/parsed_extern_fn.rs

Lines 158 to 179 in 9e09fce

pub(crate) fn rust_fn_sig_return_tokens(

&self,

swift_bridge_path: &Path,

types: &TypeDeclarations,

custom_type_definitions: &mut HashMap<String, TokenStream>,

) -> TokenStream {

let sig = &self.func.sig;

if let Some(ret) = BridgedType::new_with_return_type(&sig.output, types) {

if ret.can_be_encoded_with_zero_bytes() {

return quote! {};

}

if let Some(tokens) = ret.generate_custom_rust_ffi_type(swift_bridge_path, types) {

custom_type_definitions.insert(tokens.to_string(), tokens);

}

let ty = ret.to_ffi_compatible_rust_type(swift_bridge_path, types);

quote! { -> #ty }

} else {

todo!("Push to ParseErrors")

}

}

Here's a better example:

#[swift_bridge::bridge] mod ffi { extern "Rust" { fn a() -> Result<(u32, u32), ()> fn b() -> (u32, u32) } }

I think this would generate the (u32, u32) support types twice. (as you described in your other comment)

So, yeah, this works for now but will have to change in the future.

I still think -> TokenStream makes more sense than -> Vec<TokenStream> , especially since what we're really doing is generating support code, not just types.

Let me know if you disagree.

This is all pretty minor stuff since this is all used internally, so not a huge deal, just calling this out since changing the signature seems unnecessary and less clear to me.

Do you have thoughts?

I apologize for the late response. I'll write my opinion today or tomorrow.

I'm not understanding what you're saying?

I'm just talking about returning ->TokenStream instead of -> Vec<TokenStream>.

So, I'm essentially saying to concatenated the Vec<TokenStream> that you're creating into a single TokenStream.

Is what you're saying related to that?

I'm just talking about returning ->TokenStream instead of -> Vec.

Returning TokenStream

The custom types are managed by a HashMap<String, TokenStream>. If we return a TokenStream, the contents of the HashMap<String, TokenStream> will be something like:

key1: "# [repr (C)] pub enum ResultTupleI32U32AndSomeErrEnum { Ok (__swift_bridge__tuple_I32U32) , Err (__swift_bridge__SomeErrEnum) , }, # [repr (C)] # [doc (hidden)] pub struct __swift_bridge__tuple_I32U32 (i32 , u32) ;"
value1: # [repr (C)] pub enum ResultTupleI32U32AndSomeErrEnum { Ok (__swift_bridge__tuple_I32U32) , Err (__swift_bridge__SomeErrEnum) , }, # [repr (C)] # [doc (hidden)] pub struct __swift_bridge__tuple_I32U32 (i32 , u32) ;

key2: "# [repr (C)] # [doc (hidden)] pub struct __swift_bridge__tuple_I32U32 (i32 , u32) ;"
value2: # [repr (C)] # [doc (hidden)] pub struct __swift_bridge__tuple_I32U32 (i32 , u32) ;

Due to the double definition of __swift_bridge__tuple_I32U32(please see value1 and value2), the error occurs:

the name `__swift_bridge__tuple_I32ResultTestOpaqueRustTypeString` is defined multiple times `__swift_bridge__tuple_I32ResultTestOpaqueRustTypeString` must be defined only once in the type namespace of this module

This problem can be solved by individually managing the custom types in the HashMap<String, TokenStream>. So, we need to return a Vec<TokenStream>.

Returning Vec<TokenStream>

If we return a Vec<TokenStream>, the contents of the HashMap<String, TokenStream> will be something like:

key1: "# [repr (C)] pub enum ResultTupleI32U32AndSomeErrEnum { Ok (__swift_bridge__tuple_I32U32) , Err (__swift_bridge__SomeErrEnum) , }"
value1: # [repr (C)] pub enum ResultTupleI32U32AndSomeErrEnum { Ok (__swift_bridge__tuple_I32U32) , Err (__swift_bridge__SomeErrEnum) , }

key2: "# [repr (C)] # [doc (hidden)] pub struct __swift_bridge__tuple_I32U32 (i32 , u32) ;"
value2: # [repr (C)] # [doc (hidden)] pub struct __swift_bridge__tuple_I32U32 (i32 , u32) ;

The immediately above contents of HashMap<String, TokenStream> shouldn't cause any errors like the double definition of __swift_bridge__tuple_I32U32.

Here's an example of using HashMap<String, TokenStream>:

swift-bridge/crates/swift-bridge-ir/src/parsed_extern_fn.rs

Lines 170 to 172 in 9e09fce

if let Some(tokens) = ret.generate_custom_rust_ffi_type(swift_bridge_path, types) {

custom_type_definitions.insert(tokens.to_string(), tokens);

}

Right now, The return value(TokenStream) from generate_custom_rust_ffi_type is managed by a HashMap<String, TokenStream>.

Gotcha, thanks for explaining.

I have a feeling that we could make this a bit more clear, but this works for now since it's an internal detail.

I have a feeling that we could make this a bit more clear

I think so too, but I have no idea now. It might be good to create an issue to discuss this further.

NiwakaDev · 2023-04-24T14:33:54Z

Want to update the UI test?

Yes! I'll address this.

NiwakaDev commented Apr 23, 2023

View reviewed changes

chinedufn requested changes Apr 24, 2023

View reviewed changes

NiwakaDev mentioned this pull request Apr 24, 2023

Update incorrect-return-type.stderr #215

Merged

Support returning Result<tuple, Any Type> from Rust

d9c0261

NiwakaDev force-pushed the support_tuple_result branch from e5f28fb to d9c0261 Compare April 25, 2023 23:32

chinedufn merged commit 852ece7 into chinedufn:master May 4, 2023

NiwakaDev mentioned this pull request May 18, 2023

Support RustString in extern Swift functions #225

Merged

NiwakaDev mentioned this pull request May 25, 2023

Fix already_declared attribute for enums and structs #226

Merged

NiwakaDev mentioned this pull request Jun 19, 2023

Fix non-deterministic generation order #230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support returning `Result<tuple, Any Type>` from Rust functions #213

Support returning `Result<tuple, Any Type>` from Rust functions #213

NiwakaDev commented Apr 23, 2023

NiwakaDev Apr 23, 2023 •

edited

Loading

NiwakaDev commented Apr 23, 2023 •

edited

Loading

NiwakaDev commented Apr 23, 2023

chinedufn commented Apr 24, 2023

chinedufn left a comment

chinedufn Apr 24, 2023

NiwakaDev Apr 25, 2023 •

edited

Loading

chinedufn Apr 24, 2023

NiwakaDev Apr 25, 2023 •

edited

Loading

chinedufn Apr 26, 2023

chinedufn May 1, 2023

NiwakaDev May 1, 2023 •

edited

Loading

chinedufn May 3, 2023

NiwakaDev May 4, 2023 •

edited

Loading

NiwakaDev May 4, 2023 •

edited

Loading

chinedufn May 4, 2023

NiwakaDev May 4, 2023

NiwakaDev commented Apr 24, 2023

	pub(crate) fn rust_fn_sig_return_tokens(
	&self,
	swift_bridge_path: &Path,
	types: &TypeDeclarations,
	custom_type_definitions: &mut HashMap<String, TokenStream>,
	) -> TokenStream {
	let sig = &self.func.sig;

	if let Some(ret) = BridgedType::new_with_return_type(&sig.output, types) {
	if ret.can_be_encoded_with_zero_bytes() {
	return quote! {};
	}
	if let Some(tokens) = ret.generate_custom_rust_ffi_type(swift_bridge_path, types) {
	custom_type_definitions.insert(tokens.to_string(), tokens);
	}
	let ty = ret.to_ffi_compatible_rust_type(swift_bridge_path, types);
	quote! { -> #ty }
	} else {
	todo!("Push to ParseErrors")
	}
	}

Support returning Result<tuple, Any Type> from Rust functions #213

Support returning Result<tuple, Any Type> from Rust functions #213

Conversation

NiwakaDev commented Apr 23, 2023

NiwakaDev Apr 23, 2023 • edited Loading

Choose a reason for hiding this comment

NiwakaDev commented Apr 23, 2023 • edited Loading

NiwakaDev commented Apr 23, 2023

chinedufn commented Apr 24, 2023

chinedufn left a comment

Choose a reason for hiding this comment

chinedufn Apr 24, 2023

Choose a reason for hiding this comment

NiwakaDev Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

chinedufn Apr 24, 2023

Choose a reason for hiding this comment

NiwakaDev Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

chinedufn Apr 26, 2023

Choose a reason for hiding this comment

chinedufn May 1, 2023

Choose a reason for hiding this comment

NiwakaDev May 1, 2023 • edited Loading

Choose a reason for hiding this comment

chinedufn May 3, 2023

Choose a reason for hiding this comment

NiwakaDev May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Returning TokenStream

Returning Vec<TokenStream>

NiwakaDev May 4, 2023 • edited Loading

Choose a reason for hiding this comment

chinedufn May 4, 2023

Choose a reason for hiding this comment

NiwakaDev May 4, 2023

Choose a reason for hiding this comment

NiwakaDev commented Apr 24, 2023

Support returning `Result<tuple, Any Type>` from Rust functions #213

Support returning `Result<tuple, Any Type>` from Rust functions #213

NiwakaDev Apr 23, 2023 •

edited

Loading

NiwakaDev commented Apr 23, 2023 •

edited

Loading

NiwakaDev Apr 25, 2023 •

edited

Loading

NiwakaDev Apr 25, 2023 •

edited

Loading

NiwakaDev May 1, 2023 •

edited

Loading

NiwakaDev May 4, 2023 •

edited

Loading

Returning `TokenStream`

Returning `Vec<TokenStream>`

NiwakaDev May 4, 2023 •

edited

Loading