Skip to content

Commit

Permalink
Add support for named alternate constructors.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rfk committed Feb 17, 2021
1 parent a5b5a49 commit 5aed68d
Show file tree
Hide file tree
Showing 15 changed files with 370 additions and 57 deletions.
21 changes: 21 additions & 0 deletions docs/manual/src/udl/interfaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,27 @@ func display(list: TodoListProtocol) {
}
```

## Alternate Named Constructors

In addition to the default constructor connected to the `::new()` method, you can specify
alternate named constructors to create object instances in different ways. Each such constructor
must be given an explicit name, provided in the UDL with the `[Name]` attribute like so:

```idl
interface TodoList {
// The default constructor makes an empty list.
constructor();
// This alternate constructor makes a new TodoList from a list of string items.
[Name=new_from_items]
constructor(sequence<string> items)
...
```

For each alternate constructor, UniFFI will expose an appropriate static-method, class-method or similar
in the foreign language binding, and will connect it to the Rust method of the same name on the underlying
Rust struct.


## Concurrent Access

Since interfaces represent mutable data, uniffi has to take extra care
Expand Down
6 changes: 6 additions & 0 deletions examples/sprites/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ impl Sprite {
}
}

fn new_relative_to(reference: Point, direction: Vector) -> Sprite {
Sprite {
current_position: translate(&reference, direction),
}
}

fn get_position(&self) -> Point {
self.current_position.clone()
}
Expand Down
2 changes: 1 addition & 1 deletion examples/sprites/src/sprites.udl
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ dictionary Vector {
};

interface Sprite {
// Should be an optional, but I had to test nullable args :)
constructor(Point? initial_position);
[Name=new_relative_to] constructor(Point reference, Vector direction);
Point get_position();
void move_to(Point position);
void move_by(Vector direction);
Expand Down
8 changes: 6 additions & 2 deletions examples/sprites/tests/bindings/test_sprites.kts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ s.moveBy(Vector(-4.0, 2.0))
assert( s.getPosition() == Point(-3.0, 4.0) )

s.destroy()
try {
try {
s.moveBy(Vector(0.0, 0.0))
assert(false) { "Should not be able to call anything after `destroy`" }
} catch(e: IllegalStateException) {
assert(true)
}
}

val srel = Sprite.newRelativeTo(Point(0.0, 1.0), Vector(1.0, 1.5))
assert( srel.getPosition() == Point(1.0, 2.5) )

4 changes: 4 additions & 0 deletions examples/sprites/tests/bindings/test_sprites.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@

s.move_by(Vector(-4, 2))
assert s.get_position() == Point(-3, 4)

srel = Sprite.new_relative_to(Point(0, 1), Vector(1, 1.5))
assert srel.get_position() == Point(1, 2.5)

3 changes: 2 additions & 1 deletion examples/sprites/tests/bindings/test_sprites.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ assert( s.getPosition() == Point(x: 1, y: 2))
s.moveBy(direction: Vector(dx: -4, dy: 2))
assert( s.getPosition() == Point(x: -3, y: 4))


let srel = Sprite.newRelativeTo(reference: Point(x: 0.0, y: 1.0), direction: Vector(dx: 1, dy: 1.5))
assert( srel.getPosition() == Point(x: 1.0, y: 2.5) )
2 changes: 1 addition & 1 deletion examples/sprites/tests/test_generated_bindings.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
uniffi_macros::build_foreign_language_testcases!(
"src/sprites.udl",
[
// "tests/bindings/test_sprites.py",
"tests/bindings/test_sprites.py",
"tests/bindings/test_sprites.kts",
"tests/bindings/test_sprites.swift",
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ JSObject* {{ obj.name()|class_name_cpp(context) }}::WrapObject(
return dom::{{ obj.name()|class_name_cpp(context) }}_Binding::Wrap(aCx, this, aGivenProto);
}

{%- for cons in obj.constructors() %}
{%- match obj.primary_constructor() %}
{%- when Some with (cons) %}

/* static */
already_AddRefed<{{ obj.name()|class_name_cpp(context) }}> {{ obj.name()|class_name_cpp(context) }}::Constructor(
Expand All @@ -61,10 +62,14 @@ already_AddRefed<{{ obj.name()|class_name_cpp(context) }}> {{ obj.name()|class_n
auto result = MakeRefPtr<{{ obj.name()|class_name_cpp(context) }}>(global, handle);
return result.forget();
}
{%- when None %}
{%- endmatch %}

{%- for cons in obj.alternate_constructors() %}
MOZ_STATIC_ASSERT(false, "Sorry the gecko-js backend does not yet support alternate constructors");
{%- endfor %}

{%- for meth in obj.methods() %}

{% match meth.cpp_return_type() %}{% when Some with (type_) %}{{ type_|ret_type_cpp(context) }}{% else %}void{% endmatch %} {{ obj.name()|class_name_cpp(context) }}::{{ meth.name()|fn_name_cpp }}(
{%- for arg in meth.cpp_arguments() %}
{{ arg|arg_type_cpp(context) }} {{ arg.name() }}{%- if !loop.last %},{% endif %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ namespace {{ context.namespace()|class_name_webidl(context) }} {
{%- for obj in ci.iter_object_definitions() %}
[ChromeOnly, Exposed=Window]
interface {{ obj.name()|class_name_webidl(context) }} {
{%- for cons in obj.constructors() %}

{%- match obj.primary_constructor() %}
{%- when Some with (cons) %}
{%- if cons.throws().is_some() %}
[Throws]
{% endif %}
Expand All @@ -60,6 +62,23 @@ interface {{ obj.name()|class_name_webidl(context) }} {
{%- if !loop.last %}, {% endif %}
{%- endfor %}
);
{%- when None %}
{%- endmatch %}

{%- for cons in obj.alternate_constructors() %}
{%- if cons.throws().is_some() %}
[Throws]
{% endif %}
{{ obj.name()|class_name_webidl(context) }} {{ cons.name()|fn_name_webidl }}(
{%- for arg in cons.arguments() %}
{% if arg.optional() -%}optional{%- else -%}{%- endif %} {{ arg.webidl_type()|type_webidl(context) }} {{ arg.name() }}
{%- match arg.webidl_default_value() %}
{%- when Some with (literal) %} = {{ literal|literal_webidl }}
{%- else %}
{%- endmatch %}
{%- if !loop.last %}, {% endif %}
{%- endfor %}
);
{%- endfor %}

{% for meth in obj.methods() -%}
Expand Down
19 changes: 15 additions & 4 deletions uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ class {{ obj.name()|class_name_kt }}(
handle: Long
) : FFIObject(AtomicLong(handle)), {{ obj.name()|class_name_kt }}Interface {

{%- for cons in obj.constructors() %}
{%- match obj.primary_constructor() %}
{%- when Some with (cons) %}
constructor({% call kt::arg_list_decl(cons) -%}) :
this({% call kt::to_ffi_call(cons) %})
{%- endfor %}
{%- when None %}
{%- endmatch %}

/**
* Disconnect the object from the underlying Rust object.
Expand Down Expand Up @@ -48,12 +50,21 @@ class {{ obj.name()|class_name_kt }}(
}.let {
{{ "it"|lift_kt(return_type) }}
}

{%- when None -%}
override fun {{ meth.name()|fn_name_kt }}({% call kt::arg_list_protocol(meth) %}) =
callWithHandle {
{%- call kt::to_ffi_call_with_prefix("it", meth) %}
{%- call kt::to_ffi_call_with_prefix("it", meth) %}
}
{% endmatch %}
{% endfor %}

{% if obj.constructors().len() > 1 -%}
companion object {
{% for cons in obj.alternate_constructors() -%}
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) %})
{% endfor %}
}
{%- endif %}
}
2 changes: 1 addition & 1 deletion uniffi_bindgen/src/bindings/python/gen_python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ mod filters {
Type::Enum(name) => format!("{}({})", class_name_py(name)?, nm),
Type::Object(_) => panic!("No support for lifting objects, yet"),
Type::CallbackInterface(_) => panic!("No support for lifting callback interfaces, yet"),
Type::Error(_) => panic!("No support for lowering errors, yet"),
Type::Error(_) => panic!("No support for lifting errors, yet"),
Type::Record(_) | Type::Optional(_) | Type::Sequence(_) | Type::Map(_) => format!(
"{}.consumeInto{}()",
nm,
Expand Down
22 changes: 18 additions & 4 deletions uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
class {{ obj.name()|class_name_py }}(object):
# XXX TODO: support for multiple constructors...
{%- for cons in obj.constructors() %}
{%- match obj.primary_constructor() %}
{%- when Some with (cons) %}
def __init__(self, {% call py::arg_list_decl(cons) -%}):
{%- call py::coerce_args_extra_indent(cons) %}
self._handle = {% call py::to_ffi_call(cons) %}
{%- endfor %}
self._handle = {% call py::to_ffi_call(cons) %}\
{%- when None %}
{%- endmatch %}

def __del__(self):
rust_call_with_error(
Expand All @@ -13,6 +14,19 @@ def __del__(self):
self._handle
)

{% for cons in obj.alternate_constructors() -%}
@classmethod
def {{ cons.name()|fn_name_py }}(cls, {% call py::arg_list_decl(cons) %}):
{%- call py::coerce_args_extra_indent(cons) %}
# Call the (fallible) function before creating any half-baked object instances.
handle = {% call py::to_ffi_call(cons) %}
# Lightly yucky way to bypass the usual __init__ logic
# and just create a new instance with the required handle.
inst = cls.__new__(cls)
inst._handle = handle
return inst
{% endfor %}

{% for meth in obj.methods() -%}
{%- match meth.return_type() -%}

Expand Down
24 changes: 18 additions & 6 deletions uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,34 @@ public protocol {{ obj.name() }}Protocol {
{% endfor %}
}

public class {{ obj.name() }}: {{ obj.name() }}Protocol {
public class {{ obj.name()|class_name_swift }}: {{ obj.name() }}Protocol {
private let handle: UInt64

{%- for cons in obj.constructors() %}
public init({% call swift::arg_list_decl(cons) -%}) {% call swift::throws(cons) %} {
self.handle = {% call swift::to_ffi_call(cons) %}
private init(fromRawHandle handle: UInt64) {
self.handle = handle
}
{%- endfor %}

{%- match obj.primary_constructor() %}
{%- when Some with (cons) %}
public convenience init({% call swift::arg_list_decl(cons) -%}) {% call swift::throws(cons) %} {
self.init(fromRawHandle: {% call swift::to_ffi_call(cons) %})
}
{%- when None %}
{%- endmatch %}

deinit {
try! rustCall(InternalError.unknown()) { err in
{{ obj.ffi_object_free().name() }}(handle, err)
}
}

// TODO: Maybe merge the two templates (i.e the one with a return type and the one without)
{% for cons in obj.alternate_constructors() %}
public static func {{ cons.name()|fn_name_swift }}({% call swift::arg_list_decl(cons) %}) {% call swift::throws(cons) %} -> {{ obj.name()|class_name_swift }} {
return {{ obj.name()|class_name_swift }}(fromRawHandle: {% call swift::to_ffi_call(cons) %})
}
{% endfor %}

{# // TODO: Maybe merge the two templates (i.e the one with a return type and the one without) #}
{% for meth in obj.methods() -%}
{%- match meth.return_type() -%}

Expand Down
Loading

0 comments on commit 5aed68d

Please sign in to comment.