Constructor return types / associated functions

[bendk]

Proc-macros and UDL-based generation disagree on what a constructor should return. Proc-macros expect Arc<Self>, UDL expects Self. We've had some discussions on this one before, but now we're finally in a place where we can implement the best solution:

• Update the proc-macro code so it expects constructors to return Self
• Add support for associated functions in proc-macros. This allows proc-macro code to return Arc<Self> or any other type it wants.
• I think it should be easy to also support them in UDL, but I don't think this needs to be a requirement if it turns out to be hard.

[jplatte]

I thought you added support for c'tors returning Self for proc-macros a while ago? Associated functions (static methods in other langs) seem like an orthogonal feature, are those supported with UDL? I thought not.

[bendk]

I don't think I ever implemented that. When I wrote up #1747 I needed to write some code to workaround the differences.

UDL has never supported it. There was a PR at one point to do that, but some felt it would be better to implement associated functions rather to add a second mode for constructors. I think we're finally at the point where that would be easy, so that's why I'm proposing it now.

[mhammond]

There was a PR at one point to do that,

Are you thinking of #1063 maybe? Frustratingly, that PR was rejected because there was a fundamental objection to something being called a "constructor" returning anything other than exactly Self - even though as noted here, procmacros have always done that here :( I still wouldn't mind dusting that PR off.

[jplatte]

My 2c: Calling the constructor's output "Self" seems a bit weird in the context of Rust, but seems very reasonable for UDL to have that capability, whatever it's named.

[mhammond]

Yeah, the attribute name isn't ideal and we probably can do better. However, I think the objection was more from the foreign side - that something declared as a "constructor" could, in-fact, return an existing object - that a developer on the foreign side has a clear expectation about what the term "constructor" means, and allowing Rust to return an Arc violated that. While I can see the point, I think the utility factor outweighs that concern.

[bendk]

You can get on owned Self instance by removing an element from a HashMap. To me, this kind of blows up the whole "new" vs "existing" distinction based on return type alone. If we could implement it, I think we should allow any function (constructor, associated function, top-level function, method, ...) to return either Foo or Arc<Foo>

However, I do think implementing associated functions alongside the constructor changes would be good. I had slight reservations about the original PR because it might have caused people to define constructors for things that should really be associated functions based on the internal logic of the library.

[mhammond]

You can get on owned Self instance by removing an element from a HashMap. To me, this kind of blows up the whole "new" vs "existing" distinction based on return type alone.

I think the debate was more about the term "constructor" rather than anything about the return type. I doubt the discussion would have been the same if that term wasn't use in UDL. But even with proc-macros, bindings generate objects with "constructors" - which typically has a well-defined meaning in foreign languages. Technically they are constructors as a new foreign object is indeed created as they are called, but in a practical sense it's not as the new wrapper wraps an existing Rust object

But as above, I think this is all just largely an interesting discussion rather than anything of practical consequence - of course library authors are free to declare a "rule" for what constructors mean for that lib and I think it's perfectly fine for UniFFI to not be too opinionated in cases like this.

[bendk]

Now that we have the LowerReturn trait, I think the constructor return type part of this issue could be handled fairly simply:

• Add a new test for this in fixtures/proc-macros
• In uniffi_macros/src/object.rs, implement LowerReturn for the type itself in addition to to the current implementation of FfiConverter for Arc<T>

I'm not 100% sure, but I think that should work.