Adding Vec support for transparent structs

[rkreutz]

This PR addresses #91

Added all the shims so transparent structs can support Vec. A couple caveats:

• In order to be able to implement the Vec methods in Rust, I needed to add a #[derive(Clone)] macro to the generated struct, which means that all properties in a struct must implement Clone or be able to derive it. We could maybe only add the macro if there are any Vec usage of the struct, but my knowledge on Rust is kind of limited to do so 😅
• Vec support has only been implemented to swift_repr = struct, so classes still won't support Vec.

Great description on how to implement this in the issue @chinedufn, was very helpful.

[chinedufn]

Thanks for putting this together!

I'll review this either tonight or tomorrow.

In the meantime, can you update your PR body to include an example of what this PR now enables?

These examples help with:

1. We use these examples in our release notes
2. We can link a new contributor to an old PR and they'll be able to more quickly understand what it's doing.

Here's an example of a PR that has an example in its body.

#181

[chinedufn]

I needed to add a #[derive(Clone)] macro to the generated struct
Vec support has only been implemented to swift_repr = struct, so classes still won't support Vec.

As you alluded to, requiring Clone on all structs wouldn't be great, so we should avoid that.

Right now transparent structs can't be passed by reference.

This is because neither I, nor any other contributor, has taken the time
to think through a good way to be able to pass Swift Structs to Rust by reference.

Swift classes, however, can easily be passed to Rust by reference. This is how sharing opaque types like type SomeType works.

I'm thinking that, for now, we only support Vec<TransparentStruct> when the struct has a class representation.

For example:

#[swift_bridge::bridge]
mod ffi {
    #[swift_bridge(swift_repr = "class")]
    struct Tomato {
        field: u32
    }
}

The above bridge module has swift_repr = "class".

This would lead to us generating a Swift class instead of a Swift struct.
This should work in pretty much the exact same way as generating a class for an opaque Rust types.

#[swift_bridge::bridge]
mod ffi {
    extern "Rust" {
        // The codegen for the `struct Tomato` above should be pretty much the same as the
        // codegen for this opaque type.
        // In both cases we generate a Swift class
        // ( well, technically 3 -> https://chinedufn.github.io/swift-bridge/bridge-module/opaque-types/index.html#exposing-opaque-rust-types )
        type OpaqueTomato;
    }
}

---

swift_repr = "class" isn't supported yet. I think that we should add support for it (in a separate PR) and then make this PR support swift_repr = "class"
instead of swift_repr = "struct".

The trick would be to massage the code so that when a struct has swift_repr = "class" it re-uses the same codegen code that we currently use for opaque Rust types.

Are you up for that? I'd be happy to write up a contribution guide if that would help.

[rkreutz]

hey @chinedufn , so the issue behind me having to use #[derive(Clone)] was mostly due to Rust code, not really Swift to Rust bridge, here was the issue:

pub extern "C" fn _get(vec: *const Vec<#struct_name>, index: usize) -> #ffi_option_struct_repr {
    let vec = unsafe { &*vec };
    let val = vec.get(index).map(|v| v.clone()); // <---- I had to use .clone() here
    #ffi_option_struct_repr::from_rust_repr(val)
}

In contrast SharedEnum uses .map(|v| *v), but I can't do the same on SharedStructs because the Rust compiler complaints about unborrowing a borrowed reference (something like that, not much familiar with Rust tbh). So if there were a way to pass this value along here in Rust I could remove the Clone derivation macro. Do you have any thoughts on this?

EDIT: here is the Rust error when using *v instead of v.clone() -> error[E0507]: cannot move out of *v which is behind a shared reference

[chinedufn]

The reason that it's working for enums is that right now we automatically emit Copy, Clone for non data-carrying enums.

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

Lines 135 to 139 in 832498a

	let automatic_derives = if shared_enum.has_one_or_more_variants_with_data() {
	vec![]
	} else {
	vec![quote! {Copy}, quote! {Clone}]
	};

So, the .map(|v| *v) was using the enum's Copy implementation.

---

Copies are cheap so this is fine, but we don't want to have unnecessary .clone()s.

---

I suppose that we could land your implementation for transparent structs that explicitly implement Copy, but that would require us to add #[derive(Copy, Clone)] support to transparent structs.

Similar to #190 #194

---

I'd imagine that folks aren't using transparent structs that contain only Copy types all that often, so this is probably less useful than supporting swift_repr = "class".

Still, I would merge swift_repr = "struct" support if it was only implemented for Copy structs for now (until we eventually come up with a good way to pass swift_repr = "struct" structs by reference), and then we could worry about swift_repr = "class" structs later.

---

Does that all make sense, or no?

[rkreutz]

It does, yes. One issue though, I believe it will be very common to have String properties in structs, and because of that they wouldn't be able to derive Copy. Should we allow deriving Clone in those cases and use it? We could try adding derive(Copy) if the struct supports it, and if not use Clone, would that be of any good?

Also, I think we will have this same issue for swift_repr = "class" right? Since in Rust it would still be a struct and this same snippet would fail, no?

[rkreutz]

Also, it seems from the documentation that Clone is a supertrait of Copy and structs that would allow simple bitwise copies would use *self as the .clone() implementation, so on cases where the consumer is not using String in their structs this snippet should be pretty fast, no?

The main issue though is that starting from the release this PR would be merged, all structs exposed through the FFI would need to implement (or derive) Clone/Copy, which could be a breaking change for consumers of this library...

[chinedufn]

Thank you for taking the time to think through all of this!

---

Should we allow deriving Clone in those cases and use it?

I think we want to avoid needing to clone in order to pass things across the FFI boundary.

Copy is fine, but I don't want us to encourage rampant cloning, especially if there's an alternative (swift_repr = "class").

The main issue though is that starting from the release this PR would be merged, all structs exposed through the FFI would need to implement (or derive) Clone/Copy,

Before landing this PR we would need to only generate the Vec impl for structs that #[derive(Copy)].

So, this wouldn't be a breaking change since Clone/Copy wouldn't be a requirement (if we don't see a Copy derive we don't emit the vec support code.)

Also, I think we will have this same issue for swift_repr = "class" right? Since in Rust it would still be a struct and this same snippet would fail, no?

No because we would pass a reference to the struct to Swift, and Swift would hold this reference inside of a class.

Here's a non swift-bridge example:

struct MyStruct {
    name: String
}

let instance = MyStruct { name: "hello".to_string() }'

let instance_ptr = &instance as *const MyStruct;

// We now have a pointer to the struct that we could pass over FFI.

This could is a good example of how passing a reference to a type from Rust -> Swift looks

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

Lines 7 to 171 in c1fd717

	/// Verify that we emit Rust, Swift and C header code that allows an extern "Rust" type be used
	/// within a Vec<T>.
	mod extern_rust_type_vec_support {
	use super::*;
	fn bridge_module_tokens() -> TokenStream {
	quote! {
	mod ffi {
	extern "Rust" {
	type MyRustType;
	}
	}
	}
	}
	fn expected_rust_tokens() -> ExpectedRustTokens {
	ExpectedRustTokens::Contains(quote! {
	const _: () = {
	#[doc(hidden)]
	#[export_name = "__swift_bridge__$Vec_MyRustType$new"]
	pub extern "C" fn _new() -> *mut Vec<super::MyRustType> {
	Box::into_raw(Box::new(Vec::new()))
	}
	#[doc(hidden)]
	#[export_name = "__swift_bridge__$Vec_MyRustType$drop"]
	pub extern "C" fn _drop(vec: *mut Vec<super::MyRustType>) {
	let vec = unsafe { Box::from_raw(vec) };
	drop(vec)
	}
	#[doc(hidden)]
	#[export_name = "__swift_bridge__$Vec_MyRustType$len"]
	pub extern "C" fn _len(vec: *const Vec<super::MyRustType>) -> usize {
	unsafe { &*vec }.len()
	}
	#[doc(hidden)]
	#[export_name = "__swift_bridge__$Vec_MyRustType$get"]
	pub extern "C" fn _get(vec: *const Vec<super::MyRustType>, index: usize) -> *const super::MyRustType {
	let vec = unsafe { & *vec };
	if let Some(val) = vec.get(index) {
	val as *const super::MyRustType
	} else {
	std::ptr::null()
	}
	}
	#[doc(hidden)]
	#[export_name = "__swift_bridge__$Vec_MyRustType$get_mut"]
	pub extern "C" fn _get_mut(vec: *mut Vec<super::MyRustType>, index: usize) -> *mut super::MyRustType {
	let vec = unsafe { &mut *vec };
	if let Some(val) = vec.get_mut(index) {
	val as *mut super::MyRustType
	} else {
	std::ptr::null::<super::MyRustType>() as *mut super::MyRustType
	}
	}
	#[doc(hidden)]
	#[export_name = "__swift_bridge__$Vec_MyRustType$push"]
	pub extern "C" fn _push(vec: *mut Vec<super::MyRustType>, val: *mut super::MyRustType) {
	unsafe { &mut *vec }.push(unsafe { *Box::from_raw(val) })
	}
	#[doc(hidden)]
	#[export_name = "__swift_bridge__$Vec_MyRustType$pop"]
	pub extern "C" fn _pop(vec: *mut Vec<super::MyRustType>) -> *mut super::MyRustType {
	let vec = unsafe { &mut *vec };
	if let Some(val) = vec.pop() {
	Box::into_raw(Box::new(val))
	} else {
	std::ptr::null::<super::MyRustType>() as *mut super::MyRustType
	}
	}
	#[doc(hidden)]
	#[export_name = "__swift_bridge__$Vec_MyRustType$as_ptr"]
	pub extern "C" fn _as_ptr(vec: *const Vec<super::MyRustType>) -> *const super::MyRustType {
	unsafe { & *vec }.as_ptr()
	}
	};
	})
	}
	fn expected_swift_code() -> ExpectedSwiftCode {
	ExpectedSwiftCode::ContainsAfterTrim(
	r#"
	extension MyRustType: Vectorizable {
	public static func vecOfSelfNew() -> UnsafeMutableRawPointer {
	__swift_bridge__$Vec_MyRustType$new()
	}
	public static func vecOfSelfFree(vecPtr: UnsafeMutableRawPointer) {
	__swift_bridge__$Vec_MyRustType$drop(vecPtr)
	}
	public static func vecOfSelfPush(vecPtr: UnsafeMutableRawPointer, value: MyRustType) {
	__swift_bridge__$Vec_MyRustType$push(vecPtr, {value.isOwned = false; return value.ptr;}())
	}
	public static func vecOfSelfPop(vecPtr: UnsafeMutableRawPointer) -> Optional<Self> {
	let pointer = __swift_bridge__$Vec_MyRustType$pop(vecPtr)
	if pointer == nil {
	return nil
	} else {
	return (MyRustType(ptr: pointer!) as! Self)
	}
	}
	public static func vecOfSelfGet(vecPtr: UnsafeMutableRawPointer, index: UInt) -> Optional<MyRustTypeRef> {
	let pointer = __swift_bridge__$Vec_MyRustType$get(vecPtr, index)
	if pointer == nil {
	return nil
	} else {
	return MyRustTypeRef(ptr: pointer!)
	}
	}
	public static func vecOfSelfGetMut(vecPtr: UnsafeMutableRawPointer, index: UInt) -> Optional<MyRustTypeRefMut> {
	let pointer = __swift_bridge__$Vec_MyRustType$get_mut(vecPtr, index)
	if pointer == nil {
	return nil
	} else {
	return MyRustTypeRefMut(ptr: pointer!)
	}
	}
	public static func vecOfSelfLen(vecPtr: UnsafeMutableRawPointer) -> UInt {
	__swift_bridge__$Vec_MyRustType$len(vecPtr)
	}
	}
	"#,
	)
	}
	fn expected_c_header() -> ExpectedCHeader {
	ExpectedCHeader::ExactAfterTrim(
	r#"
	typedef struct MyRustType MyRustType;
	void __swift_bridge__$MyRustType$_free(void* self);
	void* __swift_bridge__$Vec_MyRustType$new(void);
	void __swift_bridge__$Vec_MyRustType$drop(void* vec_ptr);
	void __swift_bridge__$Vec_MyRustType$push(void* vec_ptr, void* item_ptr);
	void* __swift_bridge__$Vec_MyRustType$pop(void* vec_ptr);
	void* __swift_bridge__$Vec_MyRustType$get(void* vec_ptr, uintptr_t index);
	void* __swift_bridge__$Vec_MyRustType$get_mut(void* vec_ptr, uintptr_t index);
	uintptr_t __swift_bridge__$Vec_MyRustType$len(void* vec_ptr);
	void* __swift_bridge__$Vec_MyRustType$as_ptr(void* vec_ptr);
	"#,
	)
	}
	#[test]
	fn extern_rust_type_vec_support() {
	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();
	}
	}

.

Notice how we get a reference to the type, cast it to a pointer and return that pointer:

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

Lines 44 to 53 in c1fd717

	#[doc(hidden)]
	#[export_name = "__swift_bridge__$Vec_MyRustType$get"]
	pub extern "C" fn _get(vec: *const Vec<super::MyRustType>, index: usize) -> *const super::MyRustType {
	let vec = unsafe { & *vec };
	if let Some(val) = vec.get(index) {
	val as *const super::MyRustType
	} else {
	std::ptr::null()
	}
	}

---

Another example of passing references

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

Lines 64 to 118 in e7aef34

	/// Verify that we generate the proper code for extern "Rust" methods that returns a reference
	/// to an opaque Rust type.
	mod test_extern_rust_function_ref_opaque_rust_type_return {
	use super::*;
	fn bridge_module_tokens() -> TokenStream {
	quote! {
	mod ffi {
	extern "Rust" {
	type SomeType;
	fn some_function() -> &SomeType;
	}
	}
	}
	}
	fn expected_rust_tokens() -> ExpectedRustTokens {
	ExpectedRustTokens::Contains(quote! {
	#[export_name = "__swift_bridge__$some_function"]
	pub extern "C" fn __swift_bridge__some_function () -> *const super::SomeType {
	super::some_function() as *const super::SomeType
	}
	})
	}
	fn expected_swift_code() -> ExpectedSwiftCode {
	ExpectedSwiftCode::ContainsAfterTrim(
	r#"
	func some_function() -> SomeTypeRef {
	SomeTypeRef(ptr: __swift_bridge__$some_function())
	}
	"#,
	)
	}
	fn expected_c_header() -> ExpectedCHeader {
	ExpectedCHeader::ContainsAfterTrim(
	r#"
	void* __swift_bridge__$some_function(void);
	"#,
	)
	}
	#[test]
	fn extern_rust_fn_ref_opaque_type_return() {
	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();
	}
	}

Note that neither example requires Copy or Clone.

---

I'd say that the best solution, at least for now, would be to support vecs on swift_repr = "class" structs.

But, I'd be cool with landing you swift_repr = "struct" support if it only worked on structs that had an explicit derive(Copy, Clone) (otherwise we don't emit the vec support code for the struct).
Although I think we both agree that requiring Copy in order to bridge a struct doesn't sound very useful.

I don't think that we can land anything that relies on cloning though, since swift-bridge aims to be a high performance bridging library with either zero or as close to zero overhead as possible in all bridging scenarios.

[chinedufn]

Yeah, now that I think more about it figuring out how to best support swift_repr = "class" will require some design work.

So it might be best to land this PR with support for swift_repr = "struct" structs that derive Copy and Clone. (see this comment about #[derive(Copy, Clone)] #193 (comment)

[rkreutz]

Yeah, now that I think more about it figuring out how to best support swift_repr = "class" will require some design work.

So it might be best to land this PR with support for swift_repr = "struct" structs that derive Copy and Clone. (see this comment about #[derive(Copy, Clone)] #193 (comment)

Yeah, I was thinking about it as well, the main issue about classes and pointers is how do you make sure that the pointer in Rust is aligned with Swift, I don't see any way of checking that, more so, how do you align them if they aren't. It seems shared classes aren't supported yet as well, so probably there is no answer to this yet.

So yeah, checking if the shared struct has Copy trait might be the best approach. However this will leave people with Strings in their structs out of this change, since they can't implement Copy on them, which might be heavily used, but I'll focus on the task at hand now.

So how can I check if a shared struct implements the Copy trait? I realise I'd do the check on /swift-bridge/crates/swift-bridge-ir/src/codegen/generate_rust_tokens/shared_struct.rs but I have no idea what property I can check on SharedStruct

[chinedufn]

Yeah, I was thinking about it as well, the main issue about classes and pointers is how do you make sure that the pointer in Rust is aligned with Swift, I don't see any way of checking that, more so, how do you align them if they aren't. It seems shared classes aren't supported yet as well, so probably there is no answer to this yet.

I'm not understanding what you mean? This would essentially be an opaque pointer.

But, at any rate, I thought about this a little more and realized that swift_repr = "class" needs some dedicated design thinking. I've made the mistake in this thread of sharing some half baked ideas.. really instead I need to sit down and more deeply explore the design space and experiment a bit.

I'll make time for that at some point.

So how can I check if a shared struct implements the Copy trait?

We'd have to add support for #[derive(Copy, Clone)] on structs.

Similar to this issue about enums, but for structs instead #190 #194

If you're up for it, you could land that as a separate PR and then you'd be able to check for copy in this PR.

[rkreutz]

If you're up for it, you could land that as a separate PR

@chinedufn opened the PR for derive() here #198

and then you'd be able to check for copy in this PR.

Ideally we should check whether the trait is implemented as well, since one can implement these traits manually without the derive macro. I'll leave a TODO in the code for such cases, but I guess for now checking the derives should be good enough.

[chinedufn]

I suppose one could impl Copy for MyStruct { }, but since it's a marker trait I'm not sure if there is any value in doing it outside of the bridge module.

But yeah, it's certainly possible. If we ever wanted to handle this we could use a #[swift_bridge(Copy)] attribute to indicate that the struct is Copy.

#[swift_bridge::bridge]
mod ffi {
    #[swift_bridge(Copy)]
    struct MyStruct {}
}

impl Copy for ffi::MyStruct {}

We do something similar for opaque Rust types https://chinedufn.github.io/swift-bridge/bridge-module/opaque-types/index.html#swift_bridgecopysize

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

Lines 259 to 284 in e7aef34

	/// Verify that we can declare a generic opaque Rust type.
	mod generic_opaque_rust_type_copy {
	use super::*;
	fn bridge_module() -> TokenStream {
	quote! {
	#[swift_bridge::bridge]
	mod ffi {
	extern "Rust" {
	#[swift_bridge(Copy(6))]
	type SomeType<u32, u16>;
	}
	}
	}
	}
	fn expected_rust_tokens() -> ExpectedRustTokens {
	ExpectedRustTokens::ContainsManyAndDoesNotContainMany {
	contains: vec![
	quote! {
	const _ : () = {
	let _ : [u8 ; std :: mem :: size_of :: < super :: SomeType<u32, u16> > ()] = [0 ; 6usize] ;
	fn _assert_copy () {
	swift_bridge::copy_support::assert_copy::<super::SomeType<u32,u16>>();
	}
	}
	},

I'll leave a TODO in the code for such cases

Sounds good!

[chinedufn]

Minor request:

Mind adding /// Verify that ... doc comments to any codegen test modules that you add.

They can sometimes feel redundant but I find that they really help when trying to understand a test since it's easier to read and make sense of prose than long snake case identifiers.
Especially for new contributors that are trying to understand what's going on.

[rkreutz]

Done b08764e

[rkreutz]

hey @chinedufn I ended up opening a separate PR because of some conflicts between my fork's master, you can have a look here #199

I've added more details on the PR's description