uniffi-rs icon indicating copy to clipboard operation
uniffi-rs copied to clipboard

Swift `final class` instead of `class`

Open terhechte opened this issue 4 years ago • 2 comments

Currently, the Swift bindings generate Swift class reference types. This seems to be used in order for Swift to call deinit when the object isn't being used anymore (and thus clean up the Rust raw memory). However I don't see subclassing being used in the generated bindings. If subclassing is not required, using final class instead of class is a useful optimization as it disables dynamic dispatch for the object in question.

I'm not sure if I missed something but if not, I'd be happy to implement this once this Swift backend rewrite is merged.

┆Issue is synchronized with this Jira Task ┆Issue Number: UNIFFI-85

terhechte avatar Aug 26 '21 09:08 terhechte

Thanks for the suggestion!

If subclassing is not required

I don't think we made an explicit decision on this front, but it turns out we accidentally decided this for the Kotlin bindings because Kotlin classes are final by default.

I think I would be happy for Swift classes to be final as well, given that we haven't really thought about whether or how subclasses might inteefere with memory-safety guarantees being upheld by the case class. We can always make them open for subclassing later (in both Kotlin and Swift) with more explicit documentation about any things to be careful of when doing so.

@jhugman @badboy what do you think?

rfk avatar Aug 27 '21 05:08 rfk

Doing this for Swift would just bring it in alignment with the Kotlin approach. So to start off this would be fine.

a-s "extends" a UniFFI-generated class by wrapping it, see this code. Glean will need to do something similar to deliver the expected user-facing API. This could be done by subclassing, which might occasionally save some boilerplate (no need to manual forward existing methods), but it works just as fine by wrapping (and just forwarding the few bits one needs, also makes it easier to hide other methods).

So if anything we might discuss if there should be an option to disable finalizing a class (some marker in the UDL?). But again this is non-critical and might be discussed in a follow-up.

badboy avatar Aug 30 '21 09:08 badboy