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

Override modifier in Kotlin

Open kirillzh opened this issue 2 years ago • 5 comments

Consider this UDL interface:

interface Test {
  string to_string();
};

This generates Kotlin code that does not compile because toString() function is missing an override modifier since this is an open, member function declared in the Any class, which all classes implement in Kotlin:

interface Test {
    // This won't compile: 'toString' hides member of supertype 'Any' and needs 'override' modifier
    fun toString(): String
}

Correct way to fix generated Kotlin code is to add missing override modifier:

interface Test {
    override fun toString(): String
}

As far as I can tell, uniffi doesn't currently provide a way to generate override modifier. Would it be reasonable to add support for it, perhaps, with an UDL syntax like this?

interface Test {
  override string to_string();
};

Here's a real use case for this problem: https://github.com/bitcoindevkit/bdk-ffi/pull/154/files#r907940746. Meanwhile, an alternative workaround here is to use different API naming.

┆Issue is synchronized with this Jira Task ┆friendlyId: UNIFFI-177

kirillzh avatar Jun 28 '22 02:06 kirillzh

I don't know how to do this in general, since which functions need override is language-specific. However, a to_string method is very common and it could be nice to support. I think this would mean:

  • Having a way to flag the function. Maybe this is an attribute, or maybe we special-case the name to_string()
  • For languages that support it (which I think is all the current languages), generate a stringify function with the proper name and signature.

bendk avatar Jul 07 '22 23:07 bendk

@kirillzh Is there any situation in which toString wouldn't need an associated override in Kotlin?

If not, then @bendk, would you accept a PR that adds that behaviour to the Kotlin binding templates?

It would be akin to the existing code to do fixups for things like reserved words.

quad avatar Aug 18 '22 12:08 quad

There are only three methods on Kotlin's Any interface:

  • equals
  • hashCode
  • toString

I strongly suspect that toString is the only reasonable method for an FFI definition to override.

quad avatar Aug 18 '22 12:08 quad

(Potential) Implementation notes

  1. Add a function to CodeOracle, perhaps something like fn_name_on_base_class
  2. Implement it for each binding; kotlin will use it, python and swift won't, ruby could but overriding methods is painless in it.
  3. Use fn_name_on_base to add an override as appropriate in:

quad avatar Aug 18 '22 13:08 quad

I don't have a strong opinion here, but have some contradictory thoughts:

  • I'm not a huge fan of just "globally" hard-coding the name (ie, making toString() magically special) because that's not the right thing to do for other languages. Eg, Python would want the same method to be named __str__ - if we really want to allow "this is a special method that gives a string repr of the object", it should be possible to do that in a consistent way regardless of the binding language.
  • I'm also not convinced hashCode and equals should be ignored. @bendk and I have been chatting about allowing interfaces to be "traits", and such interfaces could be implemented either in Rust or in bindings - so Rust would be able to hold pointers to interfaces implemented in any or all languages. A logical (future, complicated) extension to this might be to allow a uniffi-based project to hold a hashmap of such interfaces. (To be clear, I don't think this is likely, but pushing uniffi's edge-cases is becoming an art form for some projects :) IOW, like the above point, a function who's intent is "do these objects compare equal?" should be something that generated override equals for Kotlin and def __eq__(...) in Python.

It would be akin to the existing code to do fixups for things like reserved words.

  • When looking at reserved words, a motivation for me was that if someone has an existing Python based project, then later wants to add support for kotlin, they should not be forced to change the interface just because they accidentally used a kotlin reserved word. To me, the same thinking applies here - if they happen to use a to_string() method, will trying to use Kotlin cause unexpected behaviour if the to_string() in the UDL is not appropriate for toString() on Any? To put it another way, would a better option be to just rename this in the kotlin binding to, say _toString() or similar?

tl;dr, my opinion is best summarized as:

  • As a matter of policy for all binding languages, we should not "accidentally" override special methods.
  • If we choose to allow special methods to be overridden, it should be necessary to opt-in in some way which allows every binding to take action (ie, an attribute [Eq] means the actual function name is ignored; Kotlin generate equals, while Python generates __eq__, etc)

mhammond avatar Aug 23 '22 01:08 mhammond