Support Hashable #140

NiwakaDev · 2023-01-19T11:56:47Z

This PR addresses #77 and introduces the #[swift_bridge(Hashable)] attribute, which is used to generate a type that conforms to Hashable protocol.

Somthing like:

// Rust 
mod ffi {
    extern "Rust" {
        #[swift_bridge(Hashable)]
        type SomeType;
    }
}

// Generated Swift
class SomeType {
    // ...
}

// SomeType inherits SomeTypeRef. So, SomeType also conforms to Hashable protocol.
extension SomeTypeRef: Hashable {
    public func hash(into hasher: inout Hasher) {
        hasher.combine(__swift_bridge__$SomeType$_hash(rhs.ptr))
    }
}

chinedufn

Looks GREAT!

Left some very minor feedback and then we can merge this.

Thanks for your awesome work.

chinedufn · 2023-01-19T12:41:22Z

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

+                                pub extern "C" fn #function_name (
+                                    this: *const super::#ty_name,
+                                ) -> u64 {
+                                    let mut s = DefaultHasher::new();


let mut s = std::collections::hash_map::DefaultHasher

chinedufn · 2023-01-19T12:42:21Z

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

+                                    ty.ty.span(),
+                                );
+                                let tokens = quote! {
+                                use std::hash::Hash;


You can move use std::hash::{Hash, Hasher} inside of the generated function so that we don't have multiple includes in the same scope when there are multiple Hashable attributes.

You can move use std::hash::{Hash, Hasher} inside of the generated function

Do you mean this?:

pub extern "C" fn #function_name ( this: *const super::#ty_name, ) -> u64 { use std::hash::{Hash, Hasher}; let mut s = std::collections::hash_map::DefaultHasher::new(); (unsafe {&*this}).hash(&mut s); s.finish() }

Oh and you can also use DefaultHasher if you want.

use std::collections::hash_map::DefaultHasher; let mut s = DefaultHasher::new();

I'm fine either way as long we we keep the use statements inside of the function so that we don't the same use multiple times in the module's root scope (since that would create a warning that we'd have to silence).

chinedufn · 2023-01-19T12:42:37Z

SwiftRustIntegrationTestRunner/SwiftRustIntegrationTestRunnerTests/OpaqueRustStructTests.swift

@@ -89,5 +89,46 @@ class OpaqueRustStructTests: XCTestCase {
            XCTAssertNotEqual(val1, val2)
        }
    }
+
+    func testOpaqueRustTypeImplHashable() throws {


Thanks for adding tests!!

chinedufn · 2023-01-19T12:44:00Z

book/src/bridge-module/opaque-types/README.md

+#[derive(Hash, PartialEq)]
+struct RustHashType(u32);
+
+impl RustHashType {


You can remove this impl, as well as the one in the Equatable example above.

I know I'm the one that introduced this, but I'm now realizing that it's a waste of space.

You can also delete the fn new in the bridge module in both this Hashable example as well as the Equatable one. Just to get rid of noise.

chinedufn · 2023-01-19T12:45:15Z

book/src/bridge-module/opaque-types/README.md

+
+#### #[swift_bridge(Hashable)]
+
+The `Hashable` attribute allows you to expose a Rust `Hash` trait via Swift's


very minor feedback -> "... expose Rust's `Hash` trait via ..."

Sorry.. actually can you make it "... expose a Rust `Hash` trait implementation via ..."

Sorry for the hassle just trying to keep things somewhat consistent.

You can ignore this feedback if you've already went and changed it to the first comment... Not a big deal for now...

Sorry.. actually can you make it "... expose a Rust Hash trait implementation via ..."

Yes!

Done!

Sorry for the hassle just trying to keep things somewhat consistent.
You can ignore this feedback if you've already went and changed it to the first comment... Not a big deal for now...

Please do not apologize...

I felt bad for nitpicking, glad you don't care.
Sweet, thanks!

chinedufn

Looks good! Once tests are passing I can merge. Thank you!!

chinedufn · 2023-01-19T14:33:55Z

book/src/bridge-module/opaque-types/README.md

 ```

 ```swift
 // In Swift

-let val1 = RustPartialEqType(5)


Oh sorry we should keep this, I just meant to get rid of the fn new

chinedufn · 2023-01-19T14:34:19Z

book/src/bridge-module/opaque-types/README.md

    }
 }

 #[derive(PartialEq)]
-struct RustPartialEqType(u32);


Oh sorry we should keep this struct RustPartialEqType(u32);. I just meant to get rid of the fn new impl and fn new bridge.

chinedufn · 2023-01-19T14:34:42Z

book/src/bridge-module/opaque-types/README.md

+}
+
+#[derive(Hash, PartialEq)]
+struct RustHashType;


RustHashType(u32)

chinedufn · 2023-01-19T14:35:00Z

book/src/bridge-module/opaque-types/README.md

+```swift
+// In Swift
+
+let val = RustHashType()


RustHashType(10)

chinedufn · 2023-01-19T14:35:15Z

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

+
+    fn expected_rust_tokens() -> ExpectedRustTokens {
+        ExpectedRustTokens::Contains(quote! {
+        use std::hash::Hash;


This is now failing. Just need to move it inside and then I can merge

Sorry...After addressing your review, I forgot to run unit tests.

NiwakaDev · 2023-01-19T14:58:35Z

Addressed your review!

chinedufn · 2023-01-19T16:18:30Z

This is great stuff, thanks again. Loving your contributions!

NiwakaDev added 2 commits January 19, 2023 20:42

Add attribute for Hashable protocol

f1e87dc

Update opaque-types documentaion

a90584c

NiwakaDev changed the title ~~Support hashable~~ Support Hashable Jan 19, 2023

NiwakaDev added 2 commits January 19, 2023 21:02

Delete unused module

a98a76e

Format code

efec3a4

chinedufn approved these changes Jan 19, 2023

View reviewed changes

NiwakaDev added 2 commits January 19, 2023 23:21

Address review

0aa7b96

Addressed review

6512e90

chinedufn approved these changes Jan 19, 2023

View reviewed changes

NiwakaDev added 2 commits January 19, 2023 23:36

Update opaque_rust_type_codegen_tests.rs

14399f4

Addressed review

606a744

chinedufn merged commit 07a15d7 into chinedufn:master Jan 19, 2023

chinedufn mentioned this pull request Jan 20, 2023

Introduce #[swift_bridge(Hashable)] attribute for opaque Rust types #77

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Hashable #140

Support Hashable #140

NiwakaDev commented Jan 19, 2023

chinedufn left a comment

chinedufn Jan 19, 2023

chinedufn Jan 19, 2023

NiwakaDev Jan 19, 2023 •

edited

Loading

chinedufn Jan 19, 2023

chinedufn Jan 19, 2023 •

edited

Loading

chinedufn Jan 19, 2023

chinedufn Jan 19, 2023

chinedufn Jan 19, 2023 •

edited

Loading

chinedufn Jan 19, 2023 •

edited

Loading

NiwakaDev Jan 19, 2023

NiwakaDev Jan 19, 2023

chinedufn Jan 19, 2023

chinedufn left a comment

chinedufn Jan 19, 2023

chinedufn Jan 19, 2023 •

edited

Loading

chinedufn Jan 19, 2023

chinedufn Jan 19, 2023

chinedufn Jan 19, 2023

NiwakaDev Jan 19, 2023

NiwakaDev commented Jan 19, 2023

chinedufn commented Jan 19, 2023


		#### #[swift_bridge(Hashable)]

		The `Hashable` attribute allows you to expose a Rust `Hash` trait via Swift's

Support Hashable #140

Support Hashable #140

Conversation

NiwakaDev commented Jan 19, 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

NiwakaDev Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinedufn Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinedufn Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

chinedufn Jan 19, 2023 • edited Loading

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 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinedufn Jan 19, 2023 • edited Loading

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

NiwakaDev commented Jan 19, 2023

chinedufn commented Jan 19, 2023

NiwakaDev Jan 19, 2023 •

edited

Loading

chinedufn Jan 19, 2023 •

edited

Loading

chinedufn Jan 19, 2023 •

edited

Loading

chinedufn Jan 19, 2023 •

edited

Loading

chinedufn Jan 19, 2023 •

edited

Loading