Decorator objects for decorating foreign language method calls in to Rust

[jhugman]

This PR introduces the concept of Delegate objects, for decorating method calls into Rust.

We add three annotations:

1. [Delegate], to mark an interface as a Delegate object.
2. [Delegate=], to specify a delegate object for the annotated interface. Such objects have a delegate argument added to each of its constructors.
3. [CallsWith=], on methods of objects with delegate objects.

Delegate objects in Kotlin are interfaces. Methods accept the delegated object and a closure representing the Rust call.

Examples are given in the docs/manual/src/udl/delegates.md file: error catching, launching on background threads, notifying a UI after a call.

This PR only includes a Kotlin implementation, so is not ready for prime time, but otherwise ready for review.

I'd like to get more feedback before adding a Swift implementation.

Feedback sought:

• is this a concept worth pursuing?
• are the names right; do the annotations do what you'd think, do they require much explanation?
• should delegates do more? or less? e.g. do they need to be more configurable with more annotations?

[jhugman]

Thanks both.

Naming

I can definitely get behind Decorator, which I'd have got from a Patterns book. The full pattern give the decorator access to the call arguments so you can transform the args before calling. This was why I shied away from that language, but happy that you're ok with it.

The delegate terminology features heavily in iOS and macOS development, as a route to customising or listening to events of a more general component.

The goal with this was to generate the code we're already writing by hand— boring repetitive code that we're already (well I am) getting wrong; useful work without getting into off-main-thread work.

Async

For the async use-case, I agree with you both: I don't think I'm presenting a general solution for async uses. This isn't a long term fix for asynchronous calls.

However, I'm not sure that the various patterns (executors, reactive extensions, Promises/Futures, async/await, coroutines, actors, handlers etc etc etc) have stabilized on any of the platforms we operate, let alone Rust, for us to pick winners. For context, async/await was in 2021's WWDC, Coroutines only stabilized in Kotlin in 2018; Rx is popular on both iOS and Android, though being supplanted by Combine. This also doesn't account for community choice and other library preference which can lag for years.

As some (but not all) of these have language support, we can't even provide a Wrapped-style of extension that could support async/await/suspend, which spread through the codebase like @ExperimentalUnsignedType annotations.

Big picture

You've both encouraged me to think about the bigger picture:

• when we have multiple consumers the same library— I don't see the FFI as the API to the library. Uniffi makes the FFI (and the Rust code) an implementation detail of the foreign language libraries we're delivering.
• what is the UDL file— I think it's a specification of the FFI translation layer. UDL itself has— or should have— extension mechanisms to let either side of the layer help with that translation.

[bendk]

Agreed that the high-level goal of reducing boring/boiler plate code is good and worth the effort. The discussion is making me wonder if there's another way to achieve it though. I don't know if the .udl file is the right or wrong place for specifying decorators to run, but the issue with generics means that there's a practical issue at least.

Also, thanks for making the delegates.md file. It's great to see the docs written with the code and even better to have examples to discuss. On the topic of the example, it's not obvious that this is more cut-and-pasty than this. In the first example, you need to decorate all the function calls in kotlin, in the second example you need to decorate them all in the .udl file.

I do think there's a reason to prefer the second and that's functions that input or return DemoRustObject. If you decorate by hand, then you also need to unwrap each parameter and wrap each return value. However, @mhammond's and @jhugman's comments made me realize that this is just the wrapper pattern. DemoRustObject is being wrapped by DemoLib.

@jhugman mentioned that, but then said "we could only wrap the whole uniffi'd object in a handwritten object that we presented to the app." That actually seems okay to me -- it would be more-or-less that first code snippet. One issue might be if you had many methods that weren't wrapped. Writing fun doNormalThing() { return demoRustObject.doNormalThing() } dosen't seem fun. However, there are ways around that:

• On Kotlin DemoApp would delegate to DemoRustObject and only override the methods that needed decorating.
• On Swift DemoApp would inherit from DemoRustObject and only override the methods that needed decorating.

If we implemented wrapping on the bindings side, does that solve this issue?

[jhugman]

Also, thanks for making the delegates.md file. It's great to see the docs written with the code and even better to have examples to discuss.

Once again, I see the wisdom of doing this first, and get feedback before writing any code.

Writing fun doNormalThing() { return demoRustObject.doNormalThing() } dosen't seem fun

Yes, you're right.

I'd go as far as saying that wrtingfun doOtherThing() { return decorator.decoratingMethod { demoRustObject.doOtherThing() } } each time I add a new method doesn't seem fun either.

[bendk]

I'd go as far as saying that wrtingfun doOtherThing() { return decorator.decoratingMethod { demoRustObject.doOtherThing() } } each time I add a new method doesn't seem fun either.

What if we extended the concept of wrapping objects to decorating methods? I'm picturing a .toml file like this:

[decorator.run-in-background.kotlin]
definition = """\
fun <T> runInBackground(rustCall: () -> T): Future<T> {
    ...
}
"""
return_type = "Future<{T}>"

This would add the definition code to the kotlin bindings code and allow methods in the UDL to be annotated with [decorator=run-in-background]

[jhugman]

Raised #1054 to divert async/threading discussions there. While this delegates/decorators may be useful for some async usages, it's not its primary motivation.

Analogy: delegates is to async, as for loops are to recursion.

[mhammond]

However, @mhammond's and @jhugman's comments made me realize that this is just the wrapper pattern. DemoRustObject is being wrapped by DemoLib.

It took me some time to get my head around this, but it's an interesting concept. Up until now, I've really just been thinking of this "wrapping" concept applying to primitive types.

But on the foreign side, there's no good reason for that limitation. Why not wrap a namespace, dictionary or interface? At first glance, this doesn't make as much sense for the rust side, but maybe it does? Particularly for the async discussions - maybe not quite as compelling as the foreign side, but there might still be value.

(No real point to this comment other than to motivate me to step back and squint a little to try and find that uniffi-ing (hah!) concept that seems to be eluding me but, if it exists, deftly solves all these discussions)

[jhugman]

@mhammond suggested rewriting a couple of components in terms of decorators.

Rewriting Nimbus and FxAClient, I'm seeing reductions of Kotlin file size by around half.

The best bit is that methods can be added or changed in the UDL and are immediately useful to the user without additional handwritten code by the library author.

The worst bit is that it highlights that the UDL documentation is not copied straight to the Kotlin interface definition.

[mhammond]

Thanks @jhugman, I think it was worthwhile to see both side of this and I certainly like the idea of reducing boilerplate!

Rewriting Nimbus and FxAClient, I'm seeing reductions of Kotlin file size by around half.

For nimbus in particular I'm not surprised - it has been written in a style that is perfectly suited for that (and don't get me wrong - 200 less lines of boilerplate is not to be sneezed at). I'd be interested to see how it works out for fxaclient, and in particular, what it will mean to try and add the same decorators to both Kotlin and Swift where that "wrapping" pattern isn't really used in the same way, and when it is, it's different across bindings.

While I support the goals of less boilerplate, I do have some reservations about this scheme. Specifically:

• I've been thinking more about Any and how that's not going to work for some languages - let's imagine that, say, Rust was also supported as a "foreign" binding (we've actually been discussing this in the context of exposing glean to our Rust code). I don't think these decorators would work in that world, nor that Any would make sense in any context in that world.

• Related to Any, I'm concerned about the cognitive overhead of allowing decorators to change the signatures of functions. If an interface method has a CallsWith attribute, you really can't trust the signature you see at all. The actual signature must be determined by understanding a method implementation declared else-where, and where that implementation may be different in different contexts.

• It's not clear to me that the decorators make sense in all possible contexts. For example, in the Nimbus example we have on_network_thread and on_db_thread - while that's very useful for how Nimbus is currently consumed by our browsers, it's not at all clear that all possible uses of Nimbus from all possible foreign languages will have the same threading concepts.

All of which makes me lean towards a belief that (a) the "core" UDL is the wrong place to be describing these concepts, and (b) that they are part of the foreign bindings context - ie, that decorators shouldn't be part of the component definition, but instead related to component consumption. I'm not sure how we should spell that (maybe a "companion" UDL which lived on the consumer side?), but I thought I should get these thoughts out of my head for further discussion.

[mhammond]

The main use-case for Any is swallowing exceptions, right? If that wasn't a requirement, you could imagine a world where decorators could be described in toml - the signatures can be inferred.

[bendk]

The main use-case for Any is swallowing exceptions, right? If that wasn't a requirement, you could imagine a world where decorators could be described in toml - the signatures can be inferred.

I think I might be confused on this one. Here's what I thought, please correct any misunderstandings:

• Any means the return type is the same as the original function
• The other option is a non-generic return type (void or i32).
• Other generics, like Deferred<T>, are not supported

[mhammond]

I think I might be confused on this one. Here's what I thought, please correct any misunderstandings:

* `Any` means the return type is the same as the original function

Yes, I mis-spoke there - I think I meant to say Any? but that's not correct either. But I do recall an example where the decorator suppressed the exception and returned null, thereby turning the declared type into a type? - and I got the impression that the decorator could actually change the result to anything - but even if that's wrong, type to type? is still a bit of a red flag for me.

But what you described is roughly what I'm after - that a decorator can't change any types, even making them nullable. With this restriction, ISTM that decorators would seem to need far less type information described because there must be an obvious mapping from the underlying interfaces (ie, could possibly be described in toml?) and that Any wouldn't be necessary because it can be trivially inferred.

[bendk]

To me, changing the signature is a requirement for many use cases. One of the main ones is turning a T into a Deferred<T>. The extra cognitive overhead seems about the same as JsonObject which will be a different types for different consumers. My concern is flexibility. One consumer, one might want a Deferred<T>, one might want a Job, and another might want a regular T.

I hope I'm not confusing the issue here too much, but I just posted an alternate proposal that moves much of the configuration to uniffi.toml. My goal is to have something concrete to compare this proposal too. I'm interested to hear people's opinions on that one.

[mhammond]

To me, changing the signature is a requirement for many use cases. One of the main ones is turning a T into a Deferred<T>.

That does make sense.

My concern is flexibility. One consumer, one might want a Deferred<T>, one might want a Job, and another might want a regular T.

Yep, I'm 100% behind that - but like JsonObject, I'd much prefer something that uses a (probably different) concrete type per binding than spelling it as "Any".

[bendk]

Related to the above point, we care about this a lot for Nimbus because we own lots of consumer code. But we already have a few other uniffi'ed components, is it worth asking our consumers there (people in Android components I imagine) if this would help their case? Keeping in mind that in that AS-AC relationship, we usually write and manage the .udl, and they review and consume it (but maybe that relationship should evolve a little now that it's easier with uniffi? I'm digressing)

When I was working on logins, I kept thinking how nice this feature could be:

• In android-components we have code to decorate all our methods with withContext
• I believe firefox-ios does something very similar with queue.async

I think this feature would handle both cases very well.

Also, logins is littered with code that transforms our structs/enums into the a-c versions (toLoginEntry() and toEncryptedLogin() in this line). These seem like perfect candidates for the type wrapping feature.

[jhugman]

I'm torn because this code as-is would make life better for the Nimbus devs.

FWIW, as Mark suggested, I re-factored Fxaclient in terms of a decorator; I didn't go as far as I did with the Nimbus refactor but I think it serves as a reasonable second example. (PersistedFirefoxAccount is half size, and hand written code doesn't need to change when the UDL is changed).

There's going to be a lot of boring Swift boiler plate to write next year, and this is a way out of a lot of it.

putting code in uniffi.toml…

There is a different proposal which does put the code in the uniffi.toml file but doesn't seem to work for the problem at hand— i.e. how does the embedding app customize the thread on which jobs are launched, or the error reporter; how might it help remove background threading in a testing environment?

Between us on zoom, @bendk and I worked out we could do it, but only by relying of changing a global state. :(. Neither of us liked the sound of that.

I think there is a part to play for the uniffi.toml that can toggle the decorator generation on and off, but I think it would be a mistake to put code into a configuration file where it's uncompilable and untestable and where it won't get any kind of syntax highlighting when editing (recall #1047 was an important followup to be done for multiple reviewers).

Additionally I think there is room for the uniffi.toml to change the spelling of how promises work or adding suspend or async to method signatures, but I think the way we want async to work will be solved by bringing together two or three concepts; for example:

• decorators so that the app developer has runtime control of thread creation,
• typedef/wrapped types to change the type of Promise/Deferred etc, and
• an [Async] attribute to add extra language features to function signatures….

But I think these interlocking features won't come together all in one go. To succeed, I think these features need to work well on their own, and unlock interesting things when combined.

As an aside, I'm confused about how Kotlin type inference is going to help here. I'm guessing that's by disallowing decorators to change the return types and error types; but again, that's doesn't appear to solve the problem.

Second aside: I quite like the idea of the DecoratedClass attribute, but it has a certain whiff of YAGNI about it. :-)

I would love if we could come to a consensus on this one, but if we can't then I would be okay with merging the current proposal and potentially changing it down the line. It seems like it would be easier to change how this works than to change how decorators are defined.

Yes, I think this is a good path forward. My suggestion would be to:

• Land it and see how useful / offensive / elegant / clunky it actually is practice. Mark it in the docs and change log as "Experimental".
• File a followup to add a switch in the uniffi.toml file to turn the behaviour off, either for all decorators in the UDL file, or just selected ones.
• File the DecoratedClass attribute as an issue, and flesh it out ready for engineering, but wait until a demand for it.
• If another proposal arises which solves the same problems then it needs to be fleshed out ready for engineering and then implemented.

[mhammond]

I think there is a part to play for the uniffi.toml that can toggle the decorator generation on and off, but I think it would be a mistake to put code into a configuration file where it's uncompilable and untestable and where it won't get any kind of syntax highlighting when editing

My main objection here is that the decorators are described in the .udl. I might still be missing something, but the semantics of what a decorator with args means really isn't clear, not what a decorator that returns anything other than Any or void means. There's also massive complexity in supporting Any and decorators as new types, which seems quite wrong too.

I would love if we could come to a consensus on this one, but if we can't then I would be okay with merging the current proposal and potentially changing it down the line.

I'm not going to try and veto that, but I think it would be a mistake - this patch is way too large to back away from. I'd much rather see a consensus and clear, significant benefits before committing to an approach.

[mhammond]

Another reason I'm keen to keep it out of the .udl is one I haven't explicitly stated. The tl;dr is that IMO, udl is doing us amazing service, but ultimately is more liability than asset. I was very sad when Ryan's PR to replace udl with macros didn't turn out, and my brief look at Diplomat shows a similar strategy could yet work - so not entrenching it further seems like a good outcome to me.

[bendk]

I would love if we could come to a consensus on this one, but if we can't then I would be okay with merging the current proposal and potentially changing it down the line.

I'm not going to try and veto that, but I think it would be a mistake - this patch is way too large to back away from. I'd much rather see a consensus and clear, significant benefits before committing to an approach.

To be clear, I was referring to my suggestion define a separate DecoratedClass rather than alter the constructor of the existing class. I still like my suggestion, and the more I think about places the more I think we might actually need it, but it seems like that change could be done down the line.

OTOH, I really want to think through the syntax around specifying the decorator class before we merge. There's a lot of code dedicated to it and it's adding new concepts to the type system. I also think that it's not needed for a first cut. I'm going to try to write a PR that takes this as a starting point shows how it would work without that code.

[bendk]

I'm going to try to rephrase my concerns and also use some code to make it concrete.

My main concern with the current code is how the decorator class is specified in the UDL. My suggestion is that we move that specification to uniffi.config for a couple reasons:

• It adds a lot of complexity to the ComponentInterface. It requires the Generic and DecoratorObject types as well as the type_t_label method. This is quite a few lines of code and more importantly it makes the type system more complicated. Now each type needs to think about generics and there's 2 new types that work very differently than the rest. Also, this approach can only handle certain generics and I don't see a path forward for handling others.
• I don't think the UDL should specify the decorator return types. I'm hoping that from the same UDL, one consumer could have in_background_thread return a Deferred<T> and another could have it return a Promise<T>. One consumer could have with_error_reporter handle exceptions and return a T? and another consumer could have it propagate exceptions and return a T. Just like with wrapper types, it seems like the UDL should treat these as somewhat abstract and let the config file specify the concrete type.

My other point is that we don't actually need to specify the decorator interface for a Kotlin-only first-cut. The only information it's giving us is the return types of the decorator functions, but we can live without that.

• In the class definition we don't actually specify the return types for the function. We just do fun foo(...) = decoratorMethod { foo_ffi(...) } and let Kotlin figure out the type.
• We use it to generate the decorator interface, but if we tweak the system slightly, we can use the decorator class itself instead.
• We also use it to generate the interface for the object itself, but that doesn't seem necessary to me. It seems like the only reason is to support delegation and the only reason we need delegation is to have a constructor with a different signature than the auto-generated one. What about making the class open and having a subclass? If that doesn't work, what about a factory function? It's very possible I'm missing something here though.

I agree that we should avoid code in the config file. The only thing I want the config file to do is to point to a decorator class that the consumer defines. If Swift needs it, that config would also specify the return types of the methods. That's it, no need for class definitions in the config.

Here's my current vision of how decorators should work. This branch makes it so the decorator class is specified in the config. It also makes the other change I recommended: rather than changing the existing class, we define a new decorated class. It has a working Kotlin implementation and some hand-wavey descriptions of how this would work on Python/Ruby/Swift.

If there's a need to get something implemented while we discuss more, here's my proposal for a minimal first cut. This keeps things closer to this PR, but removes the code for Generic, DecoratorObject and type_t_label. Instead, the consumer defines the decorator class in the same package as the bindings and we just use that class.

Both of these have an ugly hack to the testing code in order to support adding another source file to the Kotlin package. This should be easy to clean up once I understand how that testing macro is working.

[jhugman]

The tl;dr is that IMO, udl is doing us amazing service, but ultimately is more liability than asset […] - so not entrenching it further seems like a good outcome to me.

Thanks, this is useful perspective. I'm not sure if that ship has sailed or not; we've had several cracks at it so far.

I think if we do re-write the front end as a series of proc-macros, it will be a sufficiently large change that we'd be able drop features either permanently or temporarily until we worked out to support them in the new scheme.

[jhugman]

@bendk thanks for trying to square the circle here.

I'm supportive of efforts to keep this all in the code generating backends of uniffi, though I'm not optimistic for its success.

This example is a super paired down version of what we're doing in Nimbus, but hopefully it illustrates the general case of the decorator being able to change the return type and error types.

// generated, these are stand-ins for the Rust calls
fun stringMaker(): String = "string"
fun boolMaker(): Boolean = true

// handwritten re-usable decorator code.
fun <T> catchAll(method: () -> T): T? {
    return try {
        method()
    } catch(e: Throwable) {
        null
    }
}

// generated decorated method. The return type is inferred at kotlinc compile time.
fun decoratedStringMaker() = catchAll(::stringMaker)
fun decoratedBoolMaker() = catchAll(::boolMaker)

It will work in Kotlin because you can omit the return type and use the = form, and Kotlin doesn't have checked exceptions.

fun decoratedStringMaker() = catchAll(::stringMaker)

It will work in Python because Python doesn't need you to specify the return type.

It won't work in Swift unless the the code knows about how the decorator is going to change the return types or error types.

func stringMaker() throws -> String  { "string" }
func boolMaker() throws -> Bool { true }

// handwritten decorator
func catchAll<T>(_ thunk: () throws -> T) -> T? {
    do {
        return try thunk()
    } catch {
        return nil
    }
}

// generated decorated functions must know the return types at generation time.
func decoratedStringMaker() -> String? {
    catchAll(stringMaker)
}

func decoratedBoolMaker() -> Bool? {
    catchAll(boolMaker)
}

For the case of Java (which we support via Kotlin), I'm unsure about checked exceptions. I think if the decorator method throws or rethrows an exception (which #1099 makes a checked exception) then it's going to need to know the exception type.

FWIW spending some time with a Swift playground, or a Kotlin scratch file is good way of prototyping solutions to these code generation problems.

Regrettably Uniffi has to know more about the decorator that just the method names, and codegen backends don't have enough logic in them to find that out themselves.

[jhugman]

So my proposal remains the same:

• Land it and see how useful / offensive / elegant / clunky it actually in practice. Mark it in the docs and change log as "Experimental".
• File a followup to add a switch in the uniffi.toml file to turn the behaviour off, either for all decorators in the UDL file, or just selected ones.
• File the DecoratedClass attribute as an issue, and flesh it out ready for engineering, but wait until a demand for it.
• If another proposal arises which solves the same problems then it needs to be fleshed out ready for engineering and then implemented.

I'd add: if we replace the UDL with proc-macros, then we should not block that re-write while we work out how to support decorators.

Happy to receive actionable feedback on a path forward. I'd be super happy if a better solution arises in the future, but I don't think it should stop a working solution now.

We have a rough consensus that decorators as a concept are useful, and working code. Forward momentum and course correction.

[bendk]

Regrettably Uniffi has to know more about the decorator that just the method names, and codegen backends don't have enough logic in them to find that out themselves.

I think I'm not communicating my thoughts properly here. I'm not focused on what needs to be specified, but where it should be specified. I like specifying them in the config file rather than the UDL for the reasons listed at the top of this comment. The fact that you can get away with not specifying the return types for some languages is just a side benefit.

I didn't think too hard about checked exceptions, but I think the same reasoning applies to them as to the return type. I want 2 consumers to be able to use the same UDL file and handle the exceptions differently in their decorators. One can propagate the checked exceptions and one can catch and handle them.

What do you think about specifying things in the config instead of the UDL?

If you have concerns with specifying things in the config, then would my proposal where the return types are inferred work to get started? The goal is not to never specify the return types, it's just to get something working and merged as we discuss where that specification should live. I think that would let you get started using the feature on Kotlin. Python and Ruby would be fairly easy to add. I agree that Swift wouldn't be possible though. Is Swift a requirement to get started?

[jhugman]

Is Swift a requirement to get started?

Yes, Swift and Kotlin is the minimum set of languages that any feature of uniffi must support.

I think I'm not communicating my thoughts properly here. I'm not focused on what needs to be specified, but where it should be specified. I like specifying them in the config file rather than the UDL for the reasons listed at the top of this comment.

I understand this, and I think it's a good place to start. But at some point you have to look at what it looks like to specify everything that needs to be specified in the place you want to specify it.

We seem to be at deadlock here in a cycle of:

• Let's do A!
• Hmm. That does look useful, but I don't like A. We should do B.
• Hmm. I like B, but B doesn't work. A works already, we should do A.
• Well, as I've said, I don't like A and we should look at B.
• I don't think B will work, and you don't like A.

Let's try and break this cycle.

Before writing any more Rust on a competing proposal, I think it would a useful exercise for you to prototype a tiny sketch: a UDL file, a uniffi.toml for Swift and a compilable sketch of the Swift that the toml points to, the generated Swift and what it's like to use it.

I think stringMaker/boolMaker example above might be a little too toy (it only has one decorator function, and it's not collected into a protocol) but it's about the right order of complexity: it catches exceptions, and changes the return type.

The Swift must compile, the TOML must be valid, and the UDL must be valid IDL.

That exercise will give us enough of an idea of the user experience for specifying decorators in the toml in a required language we're know we're going to find hard, test some assumptions, and show us what Rust we need to write.

Let's review it in our next 1-1, and out of this PR.

Hopefully, this will get us to a point where we can both see that B is viable or not, and if it's not, then an actionable glide path to landing A.

See you after Thanksgiving.

[bendk]

What I'm not understanding is why there would be extra difficulty with the TOML file compared to the UDL file. It seems like at the very least you could use the system proposed here with Any and Any? to handle generics, but just move that to uniffi.toml. What issues do you foresee happening there?

That said, I think that we could improve on Any and instead use something similar to the type wrapping proposal. The config file could specify return values with {} replaced with the original method's return value.

Here's a stab at the stringMaker/boolMaker example. I moved both of those functions into an interface and wrapped it with a decorator class that has catchAll as its only function.

• UDL file
• uniffi.toml
• Swift code

[jhugman]

Closed, for lack of a good way forward.