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.

Sign up for GitHub

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

Jump to bottom

Support throwing initializers #287

Merged
merged 6 commits into from
Aug 15, 2024
+315 −23
Merged

Support throwing initializers #287

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bcde7b2
Support throwing initializers
NiwakaDev Aug 10, 2024
caccf78
add parse test
NiwakaDev Aug 15, 2024
0ec86ef
chore: use -> create
NiwakaDev Aug 15, 2024
9c35120
replace switch with if
NiwakaDev Aug 15, 2024
1ee9ba2
add document
NiwakaDev Aug 15, 2024
d32e604
chore
NiwakaDev Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions SwiftRustIntegrationTestRunner/SwiftRustIntegrationTestRunnerTests/ResultTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,4 +249,27 @@ class ResultTests: XCTestCase {
XCTAssertEqual(UInt32(i), value.val())
}
}

/// Verify that we can use throwing initializers defined on the Rust side.
func testThrowingInitializers() throws {
XCTContext.runActivity(named: "Should fail") {
_ in
do {
let throwingInitializer = try ThrowingInitializer(false)
} catch let error as ResultTransparentEnum {
if case .NamedField(data: -123) = error {
// This case should pass.
} else {
XCTFail()
}
} catch {
XCTFail()
}
}
XCTContext.runActivity(named: "Should succeed") {
_ in
let throwingInitializer = try! ThrowingInitializer(true)
XCTAssertEqual(throwingInitializer.val(), 123)
}
}
}
9 changes: 9 additions & 0 deletions crates/swift-bridge-ir/src/bridged_type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ pub(crate) enum TypePosition {
FnReturn(HostLang),
SharedStructField,
SwiftCallsRustAsyncOnCompleteReturnTy,
ThrowingInit(HostLang),
}

/// &[T]
Expand Down Expand Up @@ -1177,6 +1178,7 @@ impl BridgedType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
}
}
StdLibType::Null => "()".to_string(),
Expand All @@ -1193,6 +1195,7 @@ impl BridgedType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
},
StdLibType::Vec(ty) => match type_pos {
TypePosition::FnArg(func_host_lang, _) => {
Expand All @@ -1215,6 +1218,7 @@ impl BridgedType {
"UnsafeMutableRawPointer".to_string()
}
}
TypePosition::ThrowingInit(_) => unimplemented!(),
_ => {
format!(
"RustVec<{}>",
Expand Down Expand Up @@ -1243,6 +1247,7 @@ impl BridgedType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
shared_struct.ffi_name_string()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
}
}
BridgedType::Foreign(CustomBridgedType::Shared(SharedType::Enum(shared_enum))) => {
Expand All @@ -1259,6 +1264,7 @@ impl BridgedType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
}
}
}
Expand Down Expand Up @@ -1567,6 +1573,7 @@ impl BridgedType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
},
PointerKind::Mut => expression.to_string(),
},
Expand Down Expand Up @@ -1660,6 +1667,7 @@ impl BridgedType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
},
},
StdLibType::Str => match type_pos {
Expand All @@ -1677,6 +1685,7 @@ impl BridgedType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
},
StdLibType::Vec(_) => {
format!(
Expand Down
14 changes: 14 additions & 0 deletions crates/swift-bridge-ir/src/bridged_type/bridgeable_result.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::bridged_type::{BridgeableType, BridgedType, CFfiStruct, TypePosition};
use crate::parse::HostLang;
use crate::{TypeDeclarations, SWIFT_BRIDGE_PREFIX};
use proc_macro2::{Span, TokenStream};
use quote::{format_ident, quote, quote_spanned};
Expand Down Expand Up @@ -207,6 +208,7 @@ impl BuiltInResult {
}
"__private__ResultPtrAndPtr".to_string()
}
TypePosition::ThrowingInit(_) => todo!(),
}
}

Expand Down Expand Up @@ -253,6 +255,18 @@ impl BuiltInResult {
),
TypePosition::SharedStructField => todo!(),
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => todo!(),
TypePosition::ThrowingInit(lang) => {
match lang {
HostLang::Rust => format!(
"let val = {expression}; switch val.tag {{ case {c_ok_name}: self.init(ptr: val.payload.ok) case {c_err_name}: throw {err_swift_type} default: fatalError() }}",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that it's not suitable because the convert_ffi_value_to_swift_value method returns a expression.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you're communicating?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're asking whether it's okay to return two statements here instead of a single expression?

This should be fine since the ThrowingInit code here is only ever used inside of an initializer and isn't ever embedded inside of another expression.

Copy link
Collaborator Author

@NiwakaDev NiwakaDev Aug 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you're communicating?

The other return values of convert_ffi_value_to_swift_value return closures, but this case doesn't return a closure. This means a simple statement, not expression.

For examples:

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

Line 286 in 495611b

return format!("try {{ let val = {expression}; if val != nil {{ throw {err} }} else {{ return{ok} }} }}()", expression = expression, err = err, ok = ok);

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

Line 247 in 495611b

"try {{ let val = {expression}; switch val.tag {{ case {c_ok_name}: return{ok_swift_type} case {c_err_name}: throw {err_swift_type} default: fatalError() }} }}()",

The above examples return closures.

expression = expression,
c_ok_name = c_ok_name,
c_err_name = c_err_name,
err_swift_type = err_swift_type
),
HostLang::Swift => todo!(),
}
}
};
}

Expand Down
4 changes: 4 additions & 0 deletions crates/swift-bridge-ir/src/bridged_type/bridgeable_string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ impl BridgeableType for BridgedString {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
"UnsafeMutableRawPointer?".to_string()
}
TypePosition::ThrowingInit(_) => todo!(),
}
}

Expand Down Expand Up @@ -130,6 +131,7 @@ impl BridgeableType for BridgedString {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
todo!()
}
TypePosition::ThrowingInit(_) => todo!(),
}
}

Expand Down Expand Up @@ -205,6 +207,7 @@ impl BridgeableType for BridgedString {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => todo!(),
}
}

Expand Down Expand Up @@ -250,6 +253,7 @@ impl BridgeableType for BridgedString {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
format!("RustString(ptr: {}!)", expression)
}
TypePosition::ThrowingInit(_) => todo!(),
}
}

Expand Down
4 changes: 4 additions & 0 deletions crates/swift-bridge-ir/src/bridged_type/bridged_opaque_type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ impl BridgeableType for OpaqueForeignType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
}
} else {
match type_pos {
Expand All @@ -146,6 +147,7 @@ impl BridgeableType for OpaqueForeignType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
}
}
}
Expand Down Expand Up @@ -396,6 +398,7 @@ impl BridgeableType for OpaqueForeignType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
}
}
} else {
Expand All @@ -420,6 +423,7 @@ impl BridgeableType for OpaqueForeignType {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/swift-bridge-ir/src/bridged_type/bridged_option.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ impl BridgedOption {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
todo!()
}
TypePosition::ThrowingInit(_) => todo!(),
},
StdLibType::Vec(_) => {
format!(
Expand Down Expand Up @@ -397,6 +398,7 @@ impl BridgedOption {
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => {
unimplemented!()
}
TypePosition::ThrowingInit(_) => unimplemented!(),
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/swift-bridge-ir/src/bridged_type/built_in_tuple.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ impl BridgeableType for BuiltInTuple {
}
TypePosition::SharedStructField => todo!(),
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => todo!(),
TypePosition::ThrowingInit(_) => todo!(),
}
}

Expand Down Expand Up @@ -227,6 +228,7 @@ impl BridgeableType for BuiltInTuple {
}
TypePosition::SharedStructField => todo!(),
TypePosition::SwiftCallsRustAsyncOnCompleteReturnTy => todo!(),
TypePosition::ThrowingInit(_) => todo!(),
}
}

Expand Down
79 changes: 79 additions & 0 deletions crates/swift-bridge-ir/src/codegen/codegen_tests/opaque_rust_type_codegen_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,3 +527,82 @@ void* __swift_bridge__$Foo$new(void);
.test();
}
}

/// Verify that we can use a Swift class with a throwing init.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use a Swift class "create a Swift class"

mod extern_rust_class_with_throwing_init {
use super::*;

fn bridge_module_tokens() -> TokenStream {
quote! {
mod foo {
enum SomeErrEnum {
Variant1,
Variant2,
}
extern "Rust" {
type Foo;

#[swift_bridge(init)]
fn new() -> Result<Foo, SomeErrEnum>;
}
}
}
}

fn expected_rust_tokens() -> ExpectedRustTokens {
ExpectedRustTokens::Contains(quote! {
# [export_name = "__swift_bridge__$Foo$new"]
pub extern "C" fn __swift_bridge__Foo_new() -> ResultFooAndSomeErrEnum{
match super :: Foo :: new() {
Ok(ok) => ResultFooAndSomeErrEnum::Ok(Box::into_raw(Box::new({
let val: super::Foo = ok;
val
})) as *mut super::Foo),
Err(err) => ResultFooAndSomeErrEnum::Err(err.into_ffi_repr()),
}
}
})
}

const EXPECTED_SWIFT: ExpectedSwiftCode = ExpectedSwiftCode::ContainsAfterTrim(
r#"
public class Foo: FooRefMut {
var isOwned: Bool = true

public override init(ptr: UnsafeMutableRawPointer) {
super.init(ptr: ptr)
}

deinit {
if isOwned {
__swift_bridge__$Foo$_free(ptr)
}
}
}
extension Foo {
public convenience init() throws {
let val = __swift_bridge__$Foo$new(); switch val.tag { case __swift_bridge__$ResultFooAndSomeErrEnum$ResultOk: self.init(ptr: val.payload.ok) case __swift_bridge__$ResultFooAndSomeErrEnum$ResultErr: throw val.payload.err.intoSwiftRepr() default: fatalError() }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use an if statement instead of a switch so that we don't need this default: fatalError()

The default: fatalError makes it seem like this can fail, even though it can't.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. However, I don't address the others yet.

For example:

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

Line 247 in 495611b

"try {{ let val = {expression}; switch val.tag {{ case {c_ok_name}: return{ok_swift_type} case {c_err_name}: throw {err_swift_type} default: fatalError() }} }}()",

I'll address this in the next PR, or we can open it as good first issue.

}
}
"#,
);

const EXPECTED_C_HEADER: ExpectedCHeader = ExpectedCHeader::ContainsAfterTrim(
r#"
typedef enum __swift_bridge__$ResultFooAndSomeErrEnum$Tag {__swift_bridge__$ResultFooAndSomeErrEnum$ResultOk, __swift_bridge__$ResultFooAndSomeErrEnum$ResultErr} __swift_bridge__$ResultFooAndSomeErrEnum$Tag;
union __swift_bridge__$ResultFooAndSomeErrEnum$Fields {void* ok; struct __swift_bridge__$SomeErrEnum err;};
typedef struct __swift_bridge__$ResultFooAndSomeErrEnum{__swift_bridge__$ResultFooAndSomeErrEnum$Tag tag; union __swift_bridge__$ResultFooAndSomeErrEnum$Fields payload;} __swift_bridge__$ResultFooAndSomeErrEnum;
"#,
);

#[test]
fn extern_rust_class_with_throwing_init() {
CodegenTest {
bridge_module: bridge_module_tokens().into(),
expected_rust_tokens: expected_rust_tokens(),
expected_swift_code: EXPECTED_SWIFT,
expected_c_header: EXPECTED_C_HEADER,
}
.test();
}
}
34 changes: 29 additions & 5 deletions crates/swift-bridge-ir/src/codegen/generate_swift/generate_function_swift_calls_rust.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::bridged_type::{fn_arg_name, BridgeableType, BridgedType, StdLibType, TypePosition};
use crate::parse::{HostLang, TypeDeclaration};
use crate::parsed_extern_fn::FailableInitializerType;
use crate::{ParsedExternFn, TypeDeclarations, SWIFT_BRIDGE_PREFIX};
use quote::ToTokens;
use std::ops::Deref;
Expand Down Expand Up @@ -55,7 +56,13 @@ pub(super) fn gen_func_swift_calls_rust(
if function.is_copy_method_on_opaque_type() {
"public init".to_string()
} else {
if function.is_swift_failable_initializer {
if let Some(crate::parsed_extern_fn::FailableInitializerType::Throwing) =
function.swift_failable_initializer
{
"public convenience init".to_string()
} else if let Some(crate::parsed_extern_fn::FailableInitializerType::Option) =
function.swift_failable_initializer
{
"public convenience init?".to_string()
} else {
"public convenience init".to_string()
Expand All @@ -69,6 +76,12 @@ pub(super) fn gen_func_swift_calls_rust(
}
};

let maybe_throws =
if let Some(FailableInitializerType::Throwing) = function.swift_failable_initializer {
" throws"
} else {
""
};
let indentation = if function.associated_type.is_some() {
" "
} else {
Expand All @@ -84,7 +97,17 @@ pub(super) fn gen_func_swift_calls_rust(
let mut call_rust = if function.sig.asyncness.is_some() {
call_rust
} else if function.is_swift_initializer {
call_rust
if let Some(FailableInitializerType::Throwing) = function.swift_failable_initializer {
let built_in = function.return_ty_built_in(types).unwrap();
built_in.convert_ffi_value_to_swift_value(
&call_rust,
TypePosition::ThrowingInit(function.host_lang),
types,
swift_bridge_path,
)
} else {
call_rust
}
} else if let Some(built_in) = function.return_ty_built_in(types) {
built_in.convert_ffi_value_to_swift_value(
&call_rust,
Expand Down Expand Up @@ -183,12 +206,12 @@ pub(super) fn gen_func_swift_calls_rust(
if function.is_copy_method_on_opaque_type() {
call_rust = format!("self.bytes = {}", call_rust)
} else {
if function.is_swift_failable_initializer {
if let Some(FailableInitializerType::Option) = function.swift_failable_initializer {
call_rust = format!(
"guard let val = {} else {{ return nil }}; self.init(ptr: val)",
call_rust
)
} else {
} else if function.swift_failable_initializer.is_none() {
call_rust = format!("self.init(ptr: {})", call_rust)
}
}
Expand Down Expand Up @@ -315,7 +338,7 @@ return{maybe_try}await {with_checked_continuation_function_name}({{ (continuatio
)
} else {
format!(
r#"{indentation}{maybe_static_class_func}{swift_class_func_name}{maybe_generics}({params}){maybe_ret} {{
r#"{indentation}{maybe_static_class_func}{swift_class_func_name}{maybe_generics}({params}){maybe_throws}{maybe_ret} {{
{indentation} {call_rust}
{indentation}}}"#,
indentation = indentation,
Expand All @@ -325,6 +348,7 @@ return{maybe_try}await {with_checked_continuation_function_name}({{ (continuatio
params = params,
maybe_ret = maybe_return,
call_rust = call_rust,
maybe_throws = maybe_throws,
)
};

Expand Down
Loading
Loading