Add support for named alternate constructors. #387
Conversation
rfk
force-pushed
the
alternate-constructors
branch
2 times, most recently
from
d727bbe to
8a0f0b6
Compare
| fun {{ cons.name()|fn_name_kt }}({% call kt::arg_list_decl(cons) %}): {{ obj.name()|class_name_kt }} = | ||
| {{ obj.name()|class_name_kt }}({% call kt::to_ffi_call(cons) %}) |
| public init({% call swift::arg_list_decl(cons) -%}) {% call swift::throws(cons) %} { | ||
| self.handle = {% call swift::to_ffi_call(cons) %} | ||
| } |
There was a problem hiding this comment.
Nit: I'd suggest make everything go through a common initializer.
| public init({% call swift::arg_list_decl(cons) -%}) {% call swift::throws(cons) %} { | |
| self.handle = {% call swift::to_ffi_call(cons) %} | |
| } | |
| public convenience init({% call swift::arg_list_decl(cons) -%}) {% call swift::throws(cons) %} { | |
| self.init(fromRawHandle: {% call swift::to_ffi_call(cons) %}) | |
| } |
There was a problem hiding this comment.
Nice, thanks. Also, my word Swift is a conceptually-dense language, TIL about "conveience initializers" vs "designated initializers"...
| static func lift(_ handle: UInt64) throws -> {{ obj.name()|class_name_swift }} { | ||
| {{ obj.name()|class_name_swift }}(fromRawHandle: handle) | ||
| } |
There was a problem hiding this comment.
Ah, good catch, I think this is leftover from the previous factoring.
| "Attribute identity Identifier not supported: {:?}", | ||
| identity.lhs_identifier.0 | ||
| ) | ||
| } |
| Some(id) => { | ||
| let name = id.0.to_string(); | ||
| if name == "new" { | ||
| bail!("the method name \"new\" is reserved for the default constructor"); | ||
| } | ||
| name | ||
| } |
| } | ||
|
|
||
| #[test] | ||
| fn test_the_name_new_is_reserved_for_constructors() { |
There was a problem hiding this comment.
Whilst we're crossing the 👀 and dotting the 🍵 testing-wise, what happens if we have two constructors called new?
namespace test{};
interface Testing {
constructor();
[Name=new]
constructor();
};
There was a problem hiding this comment.
OK, I'll fess up...I actually started writing this exact test and then deleted it when I realized that it was likely to behave in a dumb manner (allowing duplicate names in the ComponentInterface and then giving inscrutable compile errors later on in the generated code). I'll take a look at making this error out more cleanly.
There was a problem hiding this comment.
(Surprise! Making it error out on duplicate names was actually quite straightforward and I should have just done this work the first time around 😅. Updated.)
| } | ||
| {% endmatch %} | ||
| {% endfor %} | ||
|
|
rfk
force-pushed
the
alternate-constructors
branch
from
8a0f0b6 to
5aed68d
Compare
It's a fairly common pattern for object-oriented interfaces to have static methods that act as named "factories" for producing object instances in ways that differ from the default constructor. This commit adds basic support for such named alternate constructors: * In the UDL, declare additional constructors with a [Name=Value] attribute to tell the tool what name should be exposed under. * In the Rust code, implement a corresponding method on the struct that acts as a constructor (no `self` parameter, `Self` return type). * In the foreign-language code, expect a static method on the resulting class with name matching the one provided in Rust. We continue to reserve the name "new" to refer to the default constructor. A followup could consider annotating it with an additional attribute like [DefaultConstructor] in order to let the default constructor have a different name on the Rust side.
rfk
force-pushed
the
alternate-constructors
branch
from
5aed68d to
623d32f
Compare
This is an alternative approach to factory methods, based on feedback from #384. It also fixes #37.
It's a fairly common pattern for object-oriented interfaces to have
static methods that act as named "factories" for producing object
instances in ways that differ from the default constructor.
This commit adds basic support for such named alternate constructors:
attribute to tell the tool what name should be exposed under.
that acts as a constructor (no
selfparameter,Selfreturn type).resulting class with name matching the one provided in Rust.
We continue to reserve the name "new" to refer to the default constructor.
A followup could consider annotating it with an additional attribute
like [DefaultConstructor] in order to let the default constructor have
a different name on the Rust side.