Skip to content

Commit

Permalink
Add support for factory functions and static methods.
Browse files Browse the repository at this point in the history
It's a fairly common pattern for object-oriented interfaces to have
named functions or 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 factories
by:

* Allowing owned object instances to be returned from functions
  and methods.
* Allowing interface methods to be marked as `static`.

The "owned object instances" part here is really key, since it
allows us to bypass a lot of the general concerns about returning
*references* to object instances that are outlined in #197 (and
that this commit does not at all attempt to tackle).
  • Loading branch information
rfk committed Feb 15, 2021
1 parent a5b5a49 commit af1d8a9
Show file tree
Hide file tree
Showing 19 changed files with 263 additions and 34 deletions.
19 changes: 19 additions & 0 deletions docs/manual/src/udl/interfaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,25 @@ func display(list: TodoListProtocol) {
}
```

## Static Methods

Interface methods can be marked with the `static` keyword to mark them as belonging to the interface
itself rather than to a particular instance. Static methods are commonly used to make named alternatives to
the default constructor, like this:

```idl
interface TodoList {
// The default constructor makes an empty list.
constructor();
// This static method is a shortcut for making a new TodoList from a list of items.
static TodoList new_from_items(sequence<string>)
...
```

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);
static Sprite new_relative_to(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 @@ -64,6 +64,9 @@ already_AddRefed<{{ obj.name()|class_name_cpp(context) }}> {{ obj.name()|class_n
{%- endfor %}

{%- for meth in obj.methods() %}
{% if meth.is_static() %}
MOZ_STATIC_ASSERT(false, "Sorry the gecko-js backend does not yet support static methods");
{% endif %}

{% 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() %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ interface {{ obj.name()|class_name_webidl(context) }} {
{%- if meth.throws().is_some() %}
[Throws]
{% endif %}
{%- match meth.webidl_return_type() -%}{%- when Some with (type_) %}{{ type_|type_webidl(context) }}{% when None %}void{% endmatch %} {{ meth.name()|fn_name_webidl }}(
{%- match meth.webidl_return_type() -%}{%- when Some with (type_) %}{{ type_|type_webidl(context) }}{% when None %}void{% endmatch %} {% if meth.is_static() %}static {% endif -%} {{ meth.name()|fn_name_webidl }}(
{%- for arg in meth.arguments() %}
{% if arg.optional() -%}optional{%- else -%}{%- endif %} {{ arg.webidl_type()|type_webidl(context) }} {{ arg.name() }}
{%- match arg.webidl_default_value() %}
Expand Down
35 changes: 32 additions & 3 deletions uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
public interface {{ obj.name()|class_name_kt }}Interface {
{% for meth in obj.methods() -%}
{%- if ! meth.is_static() -%}
fun {{ meth.name()|fn_name_kt }}({% call kt::arg_list_decl(meth) %})
{%- match meth.return_type() -%}
{%- when Some with (return_type) %}: {{ return_type|type_kt -}}
{%- else -%}
{%- endmatch %}
{%- endmatch -%}
{%- endif %}
{% endfor %}
}

Expand Down Expand Up @@ -38,7 +40,9 @@ class {{ obj.name()|class_name_kt }}(
}
}

{# // Instance methods #}
{% for meth in obj.methods() -%}
{%- if ! meth.is_static() -%}
{%- match meth.return_type() -%}

{%- when Some with (return_type) -%}
Expand All @@ -48,12 +52,37 @@ 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 %}
{%- endif -%}
{% endfor %}

companion object {
internal fun lift(handle: Long): {{ obj.name()|class_name_kt }} {
return {{ obj.name()|class_name_kt }}(handle)
}

{# // Static methods, if any #}
{% for meth in obj.methods() -%}
{%- if meth.is_static() -%}
{%- match meth.return_type() -%}

{%- when Some with (return_type) -%}
fun {{ meth.name()|fn_name_kt }}({% call kt::arg_list_decl(meth) %}): {{ return_type|type_kt }} {
val _retval = {% call kt::to_ffi_call(meth) %}
return {{ "_retval"|lift_kt(return_type) }}
}

{%- when None -%}
fun {{ meth.name()|fn_name_kt }}({% call kt::arg_list_decl(meth) %}) =
{% call kt::to_ffi_call(meth) %}
{% endmatch %}
{%- endif -%}
{% endfor %}
}
}
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 @@ -181,7 +181,7 @@ mod filters {
Type::Boolean => format!("(True if {} else False)", nm),
Type::String => format!("{}.consumeIntoString()", nm),
Type::Enum(name) => format!("{}({})", class_name_py(name)?, nm),
Type::Object(_) => panic!("No support for lifting objects, yet"),
Type::Object(name) => format!("liftObject({}, {})", class_name_py(name)?, nm),
Type::CallbackInterface(_) => panic!("No support for lifting callback interfaces, yet"),
Type::Error(_) => panic!("No support for lowering errors, yet"),
Type::Record(_) | Type::Optional(_) | Type::Sequence(_) | Type::Map(_) => format!(
Expand Down
19 changes: 19 additions & 0 deletions uniffi_bindgen/src/bindings/python/templates/Helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Miscellaneous "private" helpers.
#
# These are things that we need to have available for internal use,
# but that we don't want to expose as part of the public API classes,
# even as "hidden" methods.

{% if ci.iter_object_definitions().len() > 0 %}
def liftObject(cls, handle):
"""Helper to create an object instace from a handle.
This bypasses the usual __init__() logic for the given class
and just directly creates an instance with the given handle.
It's used to support factory functions and methods, which need
to return instances without invoking a (Python-level) constructor.
"""
obj = cls.__new__(cls)
obj._handle = handle
return obj
{% endif %}
19 changes: 19 additions & 0 deletions uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,24 @@ def __del__(self):
)

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

{%- when Some with (return_type) -%}
@staticmethod
def {{ meth.name()|fn_name_py }}({% call py::arg_list_decl(meth) %}):
{%- call py::coerce_args_extra_indent(meth) %}
_retval = {% call py::to_ffi_call(meth) %}
return {{ "_retval"|lift_py(return_type) }}

{%- when None -%}
@staticmethod
def {{ meth.name()|fn_name_py }}({% call py::arg_list_decl(meth) %}):
{%- call py::coerce_args_extra_indent(meth) %}
{% call py::to_ffi_call(meth) %}
{% endmatch %}

{%- else -%}
{%- match meth.return_type() -%}

{%- when Some with (return_type) -%}
Expand All @@ -27,4 +45,5 @@ def {{ meth.name()|fn_name_py }}(self, {% call py::arg_list_decl(meth) %}):
{%- call py::coerce_args_extra_indent(meth) %}
{% call py::to_ffi_call_with_prefix("self._handle", meth) %}
{% endmatch %}
{% endif %}
{% endfor %}
1 change: 1 addition & 0 deletions uniffi_bindgen/src/bindings/python/templates/wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
{% include "RustBufferTemplate.py" %}
{% include "RustBufferStream.py" %}
{% include "RustBufferBuilder.py" %}
{% include "Helpers.py" %}

# Error definitions
{% include "ErrorTemplate.py" %}
Expand Down
28 changes: 27 additions & 1 deletion uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@

public protocol {{ obj.name() }}Protocol {
{% for meth in obj.methods() -%}
{%- if ! meth.is_static() %}
func {{ meth.name()|fn_name_swift }}({% call swift::arg_list_protocol(meth) %}) {% call swift::throws(meth) -%}
{%- match meth.return_type() -%}
{%- when Some with (return_type) %} -> {{ return_type|type_swift -}}
{%- else -%}
{%- endmatch %}
{%- endif %}
{% endfor %}
}

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

init(fromRawHandle handle: UInt64) {
self.handle = handle
}

{%- 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) %}
Expand All @@ -24,8 +30,27 @@ public class {{ obj.name() }}: {{ obj.name() }}Protocol {
}
}

// TODO: Maybe merge the two templates (i.e the one with a return type and the one without)
static func lift(_ handle: UInt64) throws -> {{ obj.name()|class_name_swift }} {
{{ obj.name()|class_name_swift }}(fromRawHandle: handle)
}

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

{%- when Some with (return_type) -%}
public static func {{ meth.name()|fn_name_swift }}({% call swift::arg_list_decl(meth) %}) {% call swift::throws(meth) %} -> {{ return_type|type_swift }} {
let _retval = {% call swift::to_ffi_call(meth) %}
return {% call swift::try(meth) %} {{ "_retval"|lift_swift(return_type) }}
}

{%- when None -%}
public static func {{ meth.name()|fn_name_swift }}({% call swift::arg_list_decl(meth) %}) {% call swift::throws(meth) %} {
{% call swift::to_ffi_call(meth) %}
}
{%- endmatch %}
{%- else -%}
{%- match meth.return_type() -%}

{%- when Some with (return_type) -%}
Expand All @@ -39,5 +64,6 @@ public class {{ obj.name() }}: {{ obj.name() }}Protocol {
{% call swift::to_ffi_call_with_prefix("self.handle", meth) %}
}
{%- endmatch %}
{%- endif %}
{% endfor %}
}
3 changes: 0 additions & 3 deletions uniffi_bindgen/src/interface/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,6 @@ impl APIConverter<Function> for weedle::namespace::NamespaceMember<'_> {
impl APIConverter<Function> for weedle::namespace::OperationNamespaceMember<'_> {
fn convert(&self, ci: &mut ComponentInterface) -> Result<Function> {
let return_type = ci.resolve_return_type_expression(&self.return_type)?;
if let Some(Type::Object(_)) = return_type {
bail!("Objects cannot currently be returned from functions");
}
Ok(Function {
name: match self.identifier {
None => bail!("anonymous functions are not supported {:?}", self),
Expand Down
Loading

0 comments on commit af1d8a9

Please sign in to comment.