Support Argument-Label for Swift #156

NiwakaDev · 2023-02-04T16:25:51Z

This PR addresses #153 and introduces #[swift_bridge(label = "...")] to support Swift's Argument-Label.

Here's an example of using this:

// Rust
#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        fn test_argument_label(
            #[swift_bridge(label = "someArg")] some_arg: i32,
            another_arg: i32,
        ) -> i32;
    }
}

// Swift 
XCTAssertEqual(test_argument_label(someArg: 10, 100), 110)

chinedufn

NNNIIIICCCEEEE!!!!

Left some minor feedback but overall this is great. Awesome work!!!

chinedufn · 2023-02-04T17:25:42Z

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

@@ -92,6 +92,7 @@ pub(crate) struct ParsedExternFn {
    pub args_into: Option<Vec<Ident>>,
    /// Get one of the associated type's fields
    pub get_field: Option<GetField>,
+    pub argument_labels: Option<HashMap<Ident, LitStr>>,


Maybe just HashMap<Ident, LitStr> instead of Option?

chinedufn · 2023-02-04T17:29:36Z

crates/swift-bridge-ir/src/parse/parse_extern_mod/argument_attributes.rs

+                let value: LitStr = input.parse()?;
+                ArgumentAttr::ArgumentLabel(value)
+            }
+            _ => {


We should add a UI test file for the unrecognized argument attribute error.

Similar to

swift-bridge/crates/swift-bridge-macro/tests/ui/unrecognized-function-attribute.rs

Lines 1 to 12 in e0d3d62

//! # To Run

//! cargo test -p swift-bridge-macro -- ui trybuild=unrecognized-function-attribute.rs

#[swift_bridge::bridge]

mod ffi {

extern "Rust" {

#[swift_bridge(InvalidAttribute)]

fn some_function();

}

}

fn main() {}

swift-bridge/crates/swift-bridge-macro/tests/ui/unrecognized-function-attribute.stderr

Lines 1 to 5 in c75d6e7

error: Unrecognized attribute "InvalidAttribute".

--> tests/ui/unrecognized-function-attribute.rs:7:24

|

7 | #[swift_bridge(InvalidAttribute)]

| ^^^^^^^^^^^^^^^^

chinedufn · 2023-02-04T17:30:07Z

crates/swift-bridge-ir/src/parse/parse_extern_mod/argument_attributes.rs

+    use crate::test_utils::parse_ok;
+    use quote::{format_ident, quote};
+
+    /// Verify that we can parse a function that has a argument label.


Great test.

chinedufn · 2023-02-04T17:30:25Z

crates/swift-bridge-ir/src/parse/parse_extern_mod/function_attributes.rs

@@ -177,13 +177,17 @@ mod tests {

                    #[swift_bridge(return_into)]
                    fn some_function () -> Foo;
+
+                    fn print_hello_world();


What's this? Seems like this should be removed?

chinedufn · 2023-02-04T17:31:17Z

crates/swift-bridge-ir/src/parse/parse_extern_mod/function_attributes.rs

                }
            }
        };

        let module = parse_ok(tokens);

        assert!(module.functions[0].return_into);
+        assert_eq!(module.functions.len(), 2);


What's this? Seems like this should be removed?

Sorry...I usually add some test-codes to understand how swift-bridge works. I forgot to remove this. From now on, I'll remove them before sending a PR.

chinedufn · 2023-02-04T17:34:15Z

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

                }
            };
-
-            params.push(format!("_ {}", param))


I think we can keep the push on the outside and just change it to params.push(param).

Then in the match block we can put the _ or the argument_label.

chinedufn · 2023-02-04T17:35:47Z

SwiftRustIntegrationTestRunner/SwiftRustIntegrationTestRunnerTests/ArgumentAttributesTest.swift

+import XCTest
+@testable import SwiftRustIntegrationTestRunner
+
+/// Tests the #[swift_bridge(label = "someArg")] attribute.


/// Tests argument attributes such as `#[swift_bridge(label = "someArg")]`.

chinedufn · 2023-02-04T17:36:13Z

SwiftRustIntegrationTestRunner/SwiftRustIntegrationTestRunnerTests/ArgumentAttributesTest.swift

+        XCTAssertEqual(test_argument_label(someArg: 10, 100), 110)
+    }
+
+    func testPerformanceExample() throws {


Can delete all of this setup and tear down and performance stuff. We never use them.

chinedufn · 2023-02-04T17:39:08Z

One more thing. We need to add docs.

Can put them under here

swift-bridge/book/src/bridge-module/functions/README.md

Lines 39 to 83 in 36c90e7

    
           #### #[swift_bridge(args_into = (arg_name, another_arg_name))] 
        
           Used to name the arguments that should have `.into()` called on them when 
        
           passing them to their handler function. 
        
           One use case is for exposing a third-party type as a shared struct. 
        
           ```rust 
        
           mod pretend_this_is_some_third_party_crate { 
        
               // We want to expose this third-party struct as a shared struct. 
        
               pub struct UniqueId { 
        
                   id: u64 
        
               } 
        
           } 
        
           use pretend_this_is_some_third_party_crate::UniqueId; 
        
           fn a_function (_some_arg: UniqueId, _an_arg: UniqueId, _cool_arg: u8) { 
        
               // ... 
        
           } 
        
           mod ffi { 
        
               struct FfiUniqueId(u64); 
        
               extern "Rust" { 
        
                   // super::a_function does not take a `u64` or an `FfiUniqueId`, 
        
                   // but this still works since they both `impl Into<UniqueId>`. 
        
                   #[swift_bridge(args_into = (some_arg, an_arg))] 
        
                   fn a_function(some_arg: u64, an_arg: FfiUniqueId, cool_arg: u8); 
        
               } 
        
           } 
        
           impl From<u64> for UniqueId { 
        
               fn from(id: u64) -> UniqueId { 
        
                   UniqueId { 
        
                       id 
        
                   } 
        
               } 
        
           } 
        
           impl Into<UniqueId> for ffi::FfiUniqueId { 
        
               fn into(self) -> UniqueId { 
        
                   UniqueId(self.0) 
        
               } 
        
           } 
        
           ```

chinedufn · 2023-02-04T18:40:14Z

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

            let func_definition = match function.host_lang {
                HostLang::Rust => {
-                    gen_func_swift_calls_rust(function, &self.types, &self.swift_bridge_path)
+                    let result =


Calling this result makes it seem like we return a Result. Can we change this back?

NiwakaDev · 2023-02-05T00:55:30Z

Addressed your review! Please let me know if there are any omissions.

chinedufn · 2023-02-05T01:09:16Z

Thanks!

NiwakaDev added 3 commits February 4, 2023 23:27

Support Argument-label for Swift

54c8978

re-format

c5b5dfc

revert parse.rs

5d74d6a

chinedufn approved these changes Feb 4, 2023

View reviewed changes

chinedufn reviewed Feb 4, 2023

View reviewed changes

NiwakaDev added 3 commits February 5, 2023 09:13

address review

9959bda

address review

96d97fa

address review

211a5d6

chinedufn merged commit 3c0d00d into chinedufn:master Feb 5, 2023

chinedufn mentioned this pull request Feb 5, 2023

Support Argument-Labels #153

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Argument-Label for Swift #156

Support Argument-Label for Swift #156

NiwakaDev commented Feb 4, 2023

chinedufn left a comment

chinedufn Feb 4, 2023

chinedufn Feb 4, 2023

chinedufn Feb 4, 2023

chinedufn Feb 4, 2023

chinedufn Feb 4, 2023

NiwakaDev Feb 5, 2023

chinedufn Feb 4, 2023

chinedufn Feb 4, 2023

chinedufn Feb 4, 2023

chinedufn commented Feb 4, 2023

chinedufn Feb 4, 2023

NiwakaDev commented Feb 5, 2023

chinedufn commented Feb 5, 2023

	//! # To Run
	//! cargo test -p swift-bridge-macro -- ui trybuild=unrecognized-function-attribute.rs

	#[swift_bridge::bridge]
	mod ffi {
	extern "Rust" {
	#[swift_bridge(InvalidAttribute)]
	fn some_function();
	}
	}

	fn main() {}

	error: Unrecognized attribute "InvalidAttribute".
	--> tests/ui/unrecognized-function-attribute.rs:7:24
	\|
	7 \| #[swift_bridge(InvalidAttribute)]
	\| ^^^^^^^^^^^^^^^^

Support Argument-Label for Swift #156

Support Argument-Label for Swift #156

Conversation

NiwakaDev commented Feb 4, 2023

chinedufn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinedufn commented Feb 4, 2023

Choose a reason for hiding this comment

NiwakaDev commented Feb 5, 2023

chinedufn commented Feb 5, 2023