From 6a7745c2e94f577b96d72c8226974a84755f30fa Mon Sep 17 00:00:00 2001 From: Ryan Kelly Date: Mon, 8 Feb 2021 14:42:47 +1100 Subject: [PATCH] Add support for enums with associated data. This commit adds support for enums variants having named data fields, in a style similar to records. It also attempts to expose both "plain" enums and the new data-bearing enums to target foreign languages in an idiomatic way. Unfortunately for us, WebIDL doesn't have native syntax for this kind of data. Fortunately for us, we can fake it by using anonymous special interface methods via a syntax like: ``` [Enum] interface EnumWithData { VariantName(type1 name1, type2 name2, ...); } ``` --- README.md | 3 +- .../src/internals/lifting_and_lowering.md | 4 +- docs/manual/src/udl/enumerations.md | 24 +- examples/rondpoint/src/lib.rs | 11 +- examples/rondpoint/src/rondpoint.udl | 9 +- .../tests/bindings/test_rondpoint.kts | 19 +- .../tests/bindings/test_rondpoint.py | 24 +- .../tests/bindings/test_rondpoint.swift | 15 +- .../gecko_js/templates/SharedHeaderTemplate.h | 34 +- .../gecko_js/templates/WebIDLTemplate.webidl | 6 +- .../bindings/kotlin/templates/EnumTemplate.kt | 85 ++++- .../src/bindings/python/gen_python.rs | 21 +- .../bindings/python/templates/EnumTemplate.py | 57 ++- .../python/templates/RecordTemplate.py | 2 +- .../python/templates/RustBufferBuilder.py | 13 +- .../python/templates/RustBufferStream.py | 22 +- .../python/templates/RustBufferTemplate.py | 14 + .../swift/templates/EnumTemplate.swift | 37 +- .../swift/templates/RecordTemplate.swift | 11 +- .../src/bindings/swift/templates/macros.swift | 15 + uniffi_bindgen/src/interface/attributes.rs | 104 +++++- uniffi_bindgen/src/interface/callbacks.rs | 2 +- uniffi_bindgen/src/interface/enum_.rs | 325 +++++++++++++++++- uniffi_bindgen/src/interface/ffi.rs | 2 +- uniffi_bindgen/src/interface/function.rs | 10 +- uniffi_bindgen/src/interface/mod.rs | 121 ++++++- uniffi_bindgen/src/interface/object.rs | 9 +- uniffi_bindgen/src/interface/record.rs | 8 +- uniffi_bindgen/src/interface/types/finder.rs | 19 +- uniffi_bindgen/src/interface/types/mod.rs | 14 +- uniffi_bindgen/src/templates/EnumTemplate.rs | 45 ++- 31 files changed, 919 insertions(+), 166 deletions(-) diff --git a/README.md b/README.md index c77520f2cf..35bdb70c3a 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ Things that are implemented so far: * Primitive numeric types, equivalents to those offered by Rust (`u32`, `f64`, etc). * Strings (which are always UTF-8, like Rust's `String`). -* C-style enums (just the discriminant, no associated data). +* Enums, including enums with associated data (aka "tagged unions" or "sum types"). * C-style structs containing named fields (we call these *records*). * Sequences of all of the above (like Rust's `Vec`). * Optional instances of all of the above (like Rust's `Option`). @@ -98,7 +98,6 @@ Things that are implemented so far: Things that are not implemented yet: -* Enums with associated data. * Union types. * Efficient access to binary data (like Rust's `Vec`). * Passing object references to functions or methods. diff --git a/docs/manual/src/internals/lifting_and_lowering.md b/docs/manual/src/internals/lifting_and_lowering.md index 3c5a2dd526..86ba858d35 100644 --- a/docs/manual/src/internals/lifting_and_lowering.md +++ b/docs/manual/src/internals/lifting_and_lowering.md @@ -62,7 +62,7 @@ Calling this function from foreign language code involves the following steps: | `T?` | `RustBuffer` struct pointing to serialized bytes | | `sequence` | `RustBuffer` struct pointing to serialized bytes | | `record` | `RustBuffer` struct pointing to serialized bytes | -| `enum` | `uint32_t` indicating variant, numbered in declaration order starting from 1 | +| `enum` and `[Enum] interface` | `RustBuffer` struct pointing to serialized bytes | | `dictionary` | `RustBuffer` struct pointing to serialized bytes | | `interface` | `uint64_t` opaque integer handle | @@ -84,7 +84,7 @@ The details of this format are internal only and may change between versions of | `T?` | If null, serialized `boolean` false; if non-null, serialized `boolean` true followed by serialized `T` | | `sequence` | Serialized `i32` item count followed by serialized items; each item is a serialized `T` | | `record` | Serialized `i32` item count followed by serialized items; each item is a serialized `string` followed by a serialized `T` | -| `enum` | Serialized `u32` indicating variant, numbered in declaration order starting from 1 | +| `enum` and `[Enum] interface` | Serialized `i32` indicating variant, numbered in declaration order starting from 1, followed by the serialized values of the variant's fields in declaration order | | `dictionary` | The serialized value of each field, in declaration order | | `interface` | *Cannot currently be serialized* | diff --git a/docs/manual/src/udl/enumerations.md b/docs/manual/src/udl/enumerations.md index 9c19904cf0..f90489ce59 100644 --- a/docs/manual/src/udl/enumerations.md +++ b/docs/manual/src/udl/enumerations.md @@ -1,6 +1,7 @@ # Enumerations An enumeration defined in Rust code as + ```rust enum Animal { Dog, @@ -17,4 +18,25 @@ enum Animal { }; ``` -Note that enumerations with associated data are not yet supported. +Enumerations with associated data require a different syntax, +due to the limitations of using WebIDL as the basis for UniFFI's interface language. +An enum like this in Rust: + +```rust +enum IpAddr { + V4 {q1: u8, q2: u8, q3: u8, q4: u8}, + V6 {addr: string}, +} +``` + +Can be exposed in the UDL file with: + +```idl +[Enum] +interface IpAddr { + V4(u8 q1, u8 q2, u8 q3, u8 q4); + V6(string addr); +}; +``` + +Only enums with named fields are supported by this syntax. diff --git a/examples/rondpoint/src/lib.rs b/examples/rondpoint/src/lib.rs index faeeb3941f..de11e22984 100644 --- a/examples/rondpoint/src/lib.rs +++ b/examples/rondpoint/src/lib.rs @@ -35,6 +35,13 @@ pub enum Enumeration { Trois, } +#[derive(Debug, Clone)] +pub enum EnumerationAvecDonnees { + Zero, + Un { premier: u32 }, + Deux { premier: u32, second: String }, +} + #[allow(non_camel_case_types)] #[allow(non_snake_case)] pub struct minusculeMAJUSCULEDict { @@ -54,7 +61,9 @@ fn copie_enumerations(e: Vec) -> Vec { e } -fn copie_carte(e: HashMap) -> HashMap { +fn copie_carte( + e: HashMap, +) -> HashMap { e } diff --git a/examples/rondpoint/src/rondpoint.udl b/examples/rondpoint/src/rondpoint.udl index 75eaaa8798..d9675c47af 100644 --- a/examples/rondpoint/src/rondpoint.udl +++ b/examples/rondpoint/src/rondpoint.udl @@ -2,7 +2,7 @@ namespace rondpoint { Dictionnaire copie_dictionnaire(Dictionnaire d); Enumeration copie_enumeration(Enumeration e); sequence copie_enumerations(sequence e); - record copie_carte(record c); + record copie_carte(record c); boolean switcheroo(boolean b); }; @@ -20,6 +20,13 @@ enum Enumeration { "Trois", }; +[Enum] +interface EnumerationAvecDonnees { + Zero(); + Un(u32 premier); + Deux(u32 premier, string second); +}; + dictionary Dictionnaire { Enumeration un; boolean deux; diff --git a/examples/rondpoint/tests/bindings/test_rondpoint.kts b/examples/rondpoint/tests/bindings/test_rondpoint.kts index f09172761f..db2a3df975 100644 --- a/examples/rondpoint/tests/bindings/test_rondpoint.kts +++ b/examples/rondpoint/tests/bindings/test_rondpoint.kts @@ -6,7 +6,24 @@ assert(dico == copyDico) assert(copieEnumeration(Enumeration.DEUX) == Enumeration.DEUX) assert(copieEnumerations(listOf(Enumeration.UN, Enumeration.DEUX)) == listOf(Enumeration.UN, Enumeration.DEUX)) -assert(copieCarte(mapOf("1" to Enumeration.UN, "2" to Enumeration.DEUX)) == mapOf("1" to Enumeration.UN, "2" to Enumeration.DEUX)) +assert(copieCarte(mapOf( + "0" to EnumerationAvecDonnees.Zero, + "1" to EnumerationAvecDonnees.Un(1u), + "2" to EnumerationAvecDonnees.Deux(2u, "deux") +)) == mapOf( + "0" to EnumerationAvecDonnees.Zero, + "1" to EnumerationAvecDonnees.Un(1u), + "2" to EnumerationAvecDonnees.Deux(2u, "deux") +)) + +val var1: EnumerationAvecDonnees = EnumerationAvecDonnees.Zero +val var2: EnumerationAvecDonnees = EnumerationAvecDonnees.Un(1u) +val var3: EnumerationAvecDonnees = EnumerationAvecDonnees.Un(2u) +assert(var1 != var2) +assert(var2 != var3) +assert(var1 == EnumerationAvecDonnees.Zero) +assert(var1 != EnumerationAvecDonnees.Un(1u)) +assert(var2 == EnumerationAvecDonnees.Un(1u)) assert(switcheroo(false)) diff --git a/examples/rondpoint/tests/bindings/test_rondpoint.py b/examples/rondpoint/tests/bindings/test_rondpoint.py index 86abb17b09..ecfcc1e527 100644 --- a/examples/rondpoint/tests/bindings/test_rondpoint.py +++ b/examples/rondpoint/tests/bindings/test_rondpoint.py @@ -8,10 +8,22 @@ assert copie_enumeration(Enumeration.DEUX) == Enumeration.DEUX assert copie_enumerations([Enumeration.UN, Enumeration.DEUX]) == [Enumeration.UN, Enumeration.DEUX] -assert copie_carte({"1": Enumeration.UN, "2": Enumeration.DEUX}) == {"1": Enumeration.UN, "2": Enumeration.DEUX} +assert copie_carte({ + "0": EnumerationAvecDonnees.ZERO(), + "1": EnumerationAvecDonnees.UN(1), + "2": EnumerationAvecDonnees.DEUX(2, "deux"), +}) == { + "0": EnumerationAvecDonnees.ZERO(), + "1": EnumerationAvecDonnees.UN(1), + "2": EnumerationAvecDonnees.DEUX(2, "deux"), +} assert switcheroo(False) is True +assert EnumerationAvecDonnees.ZERO() != EnumerationAvecDonnees.UN(1) +assert EnumerationAvecDonnees.UN(1) == EnumerationAvecDonnees.UN(1) +assert EnumerationAvecDonnees.UN(1) != EnumerationAvecDonnees.UN(2) + # Test the roundtrip across the FFI. # This shows that the values we send come back in exactly the same state as we sent them. # i.e. it shows that lowering from python and lifting into rust is symmetrical with @@ -19,9 +31,9 @@ rt = Retourneur() def affirmAllerRetour(vals, identique): - for v in vals: - id_v = identique(v) - assert id_v == v, f"Round-trip failure: {v} => {id_v}" + for v in vals: + id_v = identique(v) + assert id_v == v, f"Round-trip failure: {v} => {id_v}" MIN_I8 = -1 * 2**7 MAX_I8 = 2**7 - 1 @@ -87,8 +99,8 @@ def affirmAllerRetour(vals, identique): def affirmEnchaine(vals, toString, rustyStringify=lambda v: str(v).lower()): for v in vals: - str_v = toString(v) - assert rustyStringify(v) == str_v, f"String compare error {v} => {str_v}" + str_v = toString(v) + assert rustyStringify(v) == str_v, f"String compare error {v} => {str_v}" # Test the efficacy of the string transport from rust. If this fails, but everything else # works, then things are very weird. diff --git a/examples/rondpoint/tests/bindings/test_rondpoint.swift b/examples/rondpoint/tests/bindings/test_rondpoint.swift index 7a143eddf3..756f1b8a44 100644 --- a/examples/rondpoint/tests/bindings/test_rondpoint.swift +++ b/examples/rondpoint/tests/bindings/test_rondpoint.swift @@ -6,7 +6,20 @@ assert(dico == copyDico) assert(copieEnumeration(e: .deux) == .deux) assert(copieEnumerations(e: [.un, .deux]) == [.un, .deux]) -assert(copieCarte(c: ["1": .un, "2": .deux]) == ["1": .un, "2": .deux]) +assert(copieCarte(c: + ["0": .zero, + "1": .un(premier: 1), + "2": .deux(premier: 2, second: "deux") +]) == [ + "0": .zero, + "1": .un(premier: 1), + "2": .deux(premier: 2, second: "deux") +]) + +assert(EnumerationAvecDonnees.zero != EnumerationAvecDonnees.un(premier: 1)) +assert(EnumerationAvecDonnees.un(premier: 1) == EnumerationAvecDonnees.un(premier: 1)) +assert(EnumerationAvecDonnees.un(premier: 1) != EnumerationAvecDonnees.un(premier: 2)) + assert(switcheroo(b: false)) diff --git a/uniffi_bindgen/src/bindings/gecko_js/templates/SharedHeaderTemplate.h b/uniffi_bindgen/src/bindings/gecko_js/templates/SharedHeaderTemplate.h index 532ef000a0..1783f7fcd4 100644 --- a/uniffi_bindgen/src/bindings/gecko_js/templates/SharedHeaderTemplate.h +++ b/uniffi_bindgen/src/bindings/gecko_js/templates/SharedHeaderTemplate.h @@ -29,13 +29,17 @@ namespace dom { namespace {{ context.detail_name() }} { {% for e in ci.iter_enum_definitions() %} +{% if !e.is_flat() %} +MOZ_STATIC_ASSERT(false, "Sorry the gecko-js backend does not yet support enums with associated data: {{ e.name() }}"); +{% else %} template <> -struct ViaFfi<{{ e.name()|class_name_cpp(context) }}, uint32_t> { - [[nodiscard]] static bool Lift(const uint32_t& aLowered, {{ e.name()|class_name_cpp(context) }}& aLifted) { - switch (aLowered) { +struct Serializable<{{ e.name()|class_name_cpp(context) }}> { + [[nodiscard]] static bool ReadFrom(Reader& aReader, {{ e.name()|class_name_cpp(context) }}& aValue) { + auto variant = aReader.ReadInt32(); + switch (variant) { {% for variant in e.variants() -%} case {{ loop.index }}: - aLifted = {{ e.name()|class_name_cpp(context) }}::{{ variant|enum_variant_cpp }}; + aValue = {{ e.name()|class_name_cpp(context) }}::{{ variant.name()|enum_variant_cpp }}; break; {% endfor -%} default: @@ -45,30 +49,18 @@ struct ViaFfi<{{ e.name()|class_name_cpp(context) }}, uint32_t> { return true; } - [[nodiscard]] static uint32_t Lower(const {{ e.name()|class_name_cpp(context) }}& aLifted) { - switch (aLifted) { + static void WriteInto(Writer& aWriter, const {{ e.name()|class_name_cpp(context) }}& aValue) { + switch (aValue) { {% for variant in e.variants() -%} - case {{ e.name()|class_name_cpp(context) }}::{{ variant|enum_variant_cpp }}: - return {{ loop.index }}; + case {{ e.name()|class_name_cpp(context) }}::{{ variant.name()|enum_variant_cpp }}: + aWriter.WriteInt32({{ loop.index }}); {% endfor -%} default: MOZ_ASSERT(false, "Unknown raw enum value"); } - return 0; - } -}; - -template <> -struct Serializable<{{ e.name()|class_name_cpp(context) }}> { - [[nodiscard]] static bool ReadFrom(Reader& aReader, {{ e.name()|class_name_cpp(context) }}& aValue) { - auto rawValue = aReader.ReadUInt32(); - return ViaFfi<{{ e.name()|class_name_cpp(context) }}, uint32_t>::Lift(rawValue, aValue); - } - - static void WriteInto(Writer& aWriter, const {{ e.name()|class_name_cpp(context) }}& aValue) { - aWriter.WriteUInt32(ViaFfi<{{ e.name()|class_name_cpp(context) }}, uint32_t>::Lower(aValue)); } }; +{% endif %} {% endfor %} {% for rec in ci.iter_record_definitions() -%} diff --git a/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl b/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl index c27cb89729..539820a14a 100644 --- a/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl +++ b/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl @@ -14,9 +14,13 @@ dictionary {{ rec.name()|class_name_webidl(context) }} { {% endfor %} {%- for e in ci.iter_enum_definitions() %} +{% if ! e.is_flat() %} +// Sorry the gecko-js backend does not yet support enums with associated data, +// so this probably isn't going to compile just yet... +{% endif %} enum {{ e.name()|class_name_webidl(context) }} { {% for variant in e.variants() %} - "{{ variant|enum_variant_webidl }}"{%- if !loop.last %}, {% endif %} + "{{ variant.name()|enum_variant_webidl }}"{%- if !loop.last %}, {% endif %} {% endfor %} }; {% endfor %} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt index 2a9e59d6d4..b5b55b1095 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt @@ -1,19 +1,88 @@ +{# +// Kotlin's `enum class` constuct doesn't support variants with associated data, +// but is a little nicer for consumers than its `sealed class` enum pattern. +// So, we switch here, using `enum class` for enums with no associated data +// and `sealed class` for the general case. +#} + +{% if e.is_flat() %} + enum class {{ e.name()|class_name_kt }} { - {% for variant in e.variants() %} - {{ variant|enum_variant_kt }}{% if loop.last %};{% else %},{% endif %} - {% endfor %} + {% for variant in e.variants() -%} + {{ variant.name()|enum_variant_kt }}{% if loop.last %};{% else %},{% endif %} + {%- endfor %} companion object { - internal fun lift(n: Int) = - try { values()[n - 1] } + internal fun lift(rbuf: RustBuffer.ByValue): {{ e.name()|class_name_kt }} { + return liftFromRustBuffer(rbuf) { buf -> {{ e.name()|class_name_kt }}.read(buf) } + } + + internal fun read(buf: ByteBuffer) = + try { values()[buf.getInt() - 1] } catch (e: IndexOutOfBoundsException) { throw RuntimeException("invalid enum value, something is very wrong!!", e) } + } + + internal fun lower(): RustBuffer.ByValue { + return lowerIntoRustBuffer(this, {v, buf -> v.write(buf)}) + } + + internal fun write(buf: RustBufferBuilder) { + buf.putInt(this.ordinal + 1) + } +} - internal fun read(buf: ByteBuffer) = lift(buf.getInt()) +{% else %} + +sealed class {{ e.name()|class_name_kt }} { + {% for variant in e.variants() -%} + {% if !variant.has_fields() -%} + object {{ variant.name()|class_name_kt }} : {{ e.name()|class_name_kt }}() + {% else -%} + data class {{ variant.name()|class_name_kt }}( + {% for field in variant.fields() -%} + val {{ field.name()|var_name_kt }}: {{ field.type_()|type_kt}}{% if loop.last %}{% else %}, {% endif %} + {% endfor -%} + ) : {{ e.name()|class_name_kt }}() + {%- endif %} + {% endfor %} + + companion object { + internal fun lift(rbuf: RustBuffer.ByValue): {{ e.name()|class_name_kt }} { + return liftFromRustBuffer(rbuf) { buf -> {{ e.name()|class_name_kt }}.read(buf) } + } + + internal fun read(buf: ByteBuffer): {{ e.name()|class_name_kt }} { + return when(buf.getInt()) { + {%- for variant in e.variants() %} + {{ loop.index }} -> {{ e.name()|class_name_kt }}.{{ variant.name()|class_name_kt }}{% if variant.has_fields() %}( + {% for field in variant.fields() -%} + {{ "buf"|read_kt(field.type_()) }}{% if loop.last %}{% else %},{% endif %} + {% endfor -%} + ){%- endif -%} + {%- endfor %} + else -> throw RuntimeException("invalid enum value, something is very wrong!!") + } + } } - internal fun lower() = this.ordinal + 1 + internal fun lower(): RustBuffer.ByValue { + return lowerIntoRustBuffer(this, {v, buf -> v.write(buf)}) + } - internal fun write(buf: RustBufferBuilder) = buf.putInt(this.lower()) + internal fun write(buf: RustBufferBuilder) { + when(this) { + {%- for variant in e.variants() %} + is {{ e.name()|class_name_kt }}.{{ variant.name()|class_name_kt }} -> { + buf.putInt({{ loop.index }}) + {% for field in variant.fields() -%} + {{ "(this.{})"|format(field.name())|write_kt("buf", field.type_()) }} + {% endfor -%} + } + {%- endfor %} + }.let { /* this makes the `when` an expression, which ensures it is exhaustive */ } + } } + +{% endif %} diff --git a/uniffi_bindgen/src/bindings/python/gen_python.rs b/uniffi_bindgen/src/bindings/python/gen_python.rs index 84df772d69..809b901601 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python.rs @@ -141,9 +141,10 @@ mod filters { | Type::UInt64 => format!("int({})", nm), // TODO: check max/min value Type::Float32 | Type::Float64 => format!("float({})", nm), Type::Boolean => format!("bool({})", nm), - Type::String | Type::Object(_) | Type::Error(_) | Type::Record(_) => nm.to_string(), + Type::String | Type::Object(_) | Type::Enum(_) | Type::Error(_) | Type::Record(_) => { + nm.to_string() + } Type::CallbackInterface(_) => panic!("No support for coercing callback interfaces yet"), - Type::Enum(name) => format!("{}({})", class_name_py(name)?, nm), Type::Optional(t) => format!("(None if {} is None else {})", nm, coerce_py(nm, t)?), Type::Sequence(t) => format!("list({} for x in {})", coerce_py(&"x", t)?, nm), Type::Map(t) => format!( @@ -169,11 +170,14 @@ mod filters { | Type::Float64 => nm.to_string(), Type::Boolean => format!("(1 if {} else 0)", nm), Type::String => format!("RustBuffer.allocFromString({})", nm), - Type::Enum(_) => format!("({}.value)", nm), Type::Object(_) => format!("({}._handle)", nm), Type::CallbackInterface(_) => panic!("No support for lowering callback interfaces yet"), Type::Error(_) => panic!("No support for lowering errors, yet"), - Type::Record(_) | Type::Optional(_) | Type::Sequence(_) | Type::Map(_) => format!( + Type::Enum(_) + | Type::Record(_) + | Type::Optional(_) + | Type::Sequence(_) + | Type::Map(_) => format!( "RustBuffer.allocFrom{}({})", class_name_py(&type_.canonical_name())?, nm @@ -194,11 +198,14 @@ mod filters { Type::Float32 | Type::Float64 => format!("float({})", nm), 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::CallbackInterface(_) => panic!("No support for lifting callback interfaces, yet"), - Type::Error(_) => panic!("No support for lifting errors, yet"), - Type::Record(_) | Type::Optional(_) | Type::Sequence(_) | Type::Map(_) => format!( + Type::Error(_) => panic!("No support for lowering errors, yet"), + Type::Enum(_) + | Type::Record(_) + | Type::Optional(_) + | Type::Sequence(_) + | Type::Map(_) => format!( "{}.consumeInto{}()", nm, class_name_py(&type_.canonical_name())? diff --git a/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py b/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py index 73d56578f0..ed98a6732f 100644 --- a/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py @@ -1,4 +1,59 @@ +{# +# Python has a built-in `enum` module which is nice to use, but doesn't support +# variants with associated data. So, we switch here, and generate a stdlib `enum` +# when none of the variants have associated data, or a generic nested-class +# construct when they do. +#} +{% if e.is_flat() %} + class {{ e.name()|class_name_py }}(enum.Enum): {% for variant in e.variants() -%} - {{ variant|enum_name_py }} = {{ loop.index }} + {{ variant.name()|enum_name_py }} = {{ loop.index }} {% endfor %} + +{% else %} + +class {{ e.name()|class_name_py }}(object): + def __init__(self): + raise RuntimeError("{{ e.name()|class_name_py }} cannot be instantiated directly") + + # Each enum variant is a nested class of the enum itself. + {% for variant in e.variants() -%} + class {{ variant.name()|enum_name_py }}(object): + def __init__(self,{% for field in variant.fields() %}{{ field.name()|var_name_py }}{% if loop.last %}{% else %}, {% endif %}{% endfor %}): + {% if variant.has_fields() %} + {%- for field in variant.fields() %} + self.{{ field.name()|var_name_py }} = {{ field.name()|var_name_py }} + {%- endfor %} + {% else %} + pass + {% endif %} + + def __str__(self): + return "{{ e.name()|class_name_py }}.{{ variant.name()|enum_name_py }}({% for field in variant.fields() %}{{ field.name() }}={}{% if loop.last %}{% else %}, {% endif %}{% endfor %})".format({% for field in variant.fields() %}self.{{ field.name() }}{% if loop.last %}{% else %}, {% endif %}{% endfor %}) + + def __eq__(self, other): + if not other.is_{{ variant.name()|var_name_py }}(): + return False + {%- for field in variant.fields() %} + if self.{{ field.name()|var_name_py }} != other.{{ field.name()|var_name_py }}: + return False + {%- endfor %} + return True + {% endfor %} + + # For each variant, we have an `is_NAME` method for easily checking + # whether an instance is that variant. + {% for variant in e.variants() -%} + def is_{{ variant.name()|var_name_py }}(self): + return isinstance(self, {{ e.name()|class_name_py }}.{{ variant.name()|enum_name_py }}) + {% endfor %} + +# Now, a little trick - we make each nested variant class be a subclass of the main +# enum class, so that method calls and instance checks etc will work intuitively. +# We might be able to do this a little more neatly with a metaclass, but this'll do. +{% for variant in e.variants() -%} +{{ e.name()|class_name_py }}.{{ variant.name()|enum_name_py }} = type("{{ e.name()|class_name_py }}.{{ variant.name()|enum_name_py }}", ({{ e.name()|class_name_py }}.{{variant.name()|enum_name_py}}, {{ e.name()|class_name_py }},), {}) +{% endfor %} + +{% endif %} diff --git a/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py b/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py index e399139454..b163de33b9 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py @@ -11,5 +11,5 @@ def __eq__(self, other): {%- for field in rec.fields() %} if self.{{ field.name()|var_name_py }} != other.{{ field.name()|var_name_py }}: return False - return True {%- endfor %} + return True diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferBuilder.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferBuilder.py index 98722dfca4..cedecc57e8 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferBuilder.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferBuilder.py @@ -97,7 +97,7 @@ def writeF64(self, v): {% when Type::Boolean -%} def writeBool(self, v): - self._pack_into(1, ">b", 0 if v else 1) + self._pack_into(1, ">b", 1 if v else 0) {% when Type::String -%} @@ -128,10 +128,21 @@ def write{{ canonical_type_name }}(self): raise InternalError("RustBufferStream.write() not implemented yet for {{ canonical_type_name }}") {% when Type::Enum with (enum_name) -%} + {%- let e = ci.get_enum_definition(enum_name).unwrap() -%} # The Enum type {{ enum_name }}. def write{{ canonical_type_name }}(self, v): + {%- if e.is_flat() %} self._pack_into(4, ">i", v.value) + {%- else -%} + {%- for variant in e.variants() %} + if v.is_{{ variant.name()|var_name_py }}(): + self._pack_into(4, ">i", {{ loop.index }}) + {%- for field in variant.fields() %} + self.write{{ field.type_().canonical_name()|class_name_py }}(v.{{ field.name() }}) + {%- endfor %} + {%- endfor %} + {%- endif %} {% when Type::Record with (record_name) -%} {%- let rec = ci.get_record_definition(record_name).unwrap() -%} diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferStream.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferStream.py index 78bcea88f6..c4bc85f38e 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferStream.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferStream.py @@ -125,12 +125,28 @@ def read{{ canonical_type_name }}(self): raise InternalError("RustBufferStream.read not implemented yet for {{ canonical_type_name }}") {% when Type::Enum with (enum_name) -%} + {%- let e = ci.get_enum_definition(enum_name).unwrap() -%} # The Enum type {{ enum_name }}. def read{{ canonical_type_name }}(self): - return {{ enum_name|class_name_py }}( - self._unpack_from(4, ">i") - ) + variant = self._unpack_from(4, ">i") + {% if e.is_flat() -%} + return {{ enum_name|class_name_py }}(variant) + {%- else -%} + {%- for variant in e.variants() %} + if variant == {{ loop.index }}: + {%- if variant.has_fields() %} + return {{ enum_name|class_name_py }}.{{ variant.name()|enum_name_py }}( + {%- for field in variant.fields() %} + self.read{{ field.type_().canonical_name()|class_name_py }}(){% if loop.last %}{% else %},{% endif %} + {%- endfor %} + ) + {%- else %} + return {{ enum_name|class_name_py }}.{{ variant.name()|enum_name_py }}() + {% endif %} + {%- endfor %} + raise InternalError("Unexpected variant tag for {{ canonical_type_name }}") + {%- endif %} {% when Type::Record with (record_name) -%} {%- let rec = ci.get_record_definition(record_name).unwrap() -%} diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py index 6baac71ddc..df4a428ad1 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py @@ -82,6 +82,20 @@ def consumeIntoString(self): {%- let rec = ci.get_record_definition(record_name).unwrap() -%} # The Record type {{ record_name }}. + @staticmethod + def allocFrom{{ canonical_type_name }}(v): + with RustBuffer.allocWithBuilder() as builder: + builder.write{{ canonical_type_name }}(v) + return builder.finalize() + + def consumeInto{{ canonical_type_name }}(self): + with self.consumeWithStream() as stream: + return stream.read{{ canonical_type_name }}() + + {% when Type::Enum with (enum_name) -%} + {%- let e = ci.get_enum_definition(enum_name).unwrap() -%} + # The Enum type {{ enum_name }}. + @staticmethod def allocFrom{{ canonical_type_name }}(v): with RustBuffer.allocWithBuilder() as builder: diff --git a/uniffi_bindgen/src/bindings/swift/templates/EnumTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/EnumTemplate.swift index c18de4c099..4eac959be8 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/EnumTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/EnumTemplate.swift @@ -1,30 +1,39 @@ -public enum {{ e.name()|class_name_swift }}: ViaFfi { + +// Note that we don't yet support `indirect` for enums. +// See https://github.com/mozilla/uniffi-rs/issues/396 for further discussion. +public enum {{ e.name()|class_name_swift }}: ViaFfiUsingByteBuffer, ViaFfi, Equatable { {% for variant in e.variants() %} - case {{ variant|enum_variant_swift }} + case {{ variant.name()|enum_variant_swift }}{% if variant.fields().len() > 0 %}({% call swift::field_list_decl(variant) %}){% endif -%} {% endfor %} static func read(from buf: Reader) throws -> {{ e.name()|class_name_swift }} { - return try {{ e.name()|class_name_swift }}.lift(UInt32.read(from: buf)) - } - - static func lift(_ number: UInt32) throws -> {{ e.name()|class_name_swift }} { - switch number { + let variant: Int32 = try buf.readInt() + switch variant { {% for variant in e.variants() %} - case {{ loop.index }}: return .{{ variant|enum_variant_swift }} + case {{ loop.index }}: return .{{ variant.name()|enum_variant_swift }}{% if variant.has_fields() -%}( + {% for field in variant.fields() -%} + {{ field.name()|var_name_swift }}: try {{ "buf"|read_swift(field.type_()) }}{% if loop.last %}{% else %},{% endif %} + {% endfor -%} + ){% endif -%} {% endfor %} default: throw InternalError.unexpectedEnumCase } } func write(into buf: Writer) { - self.lower().write(into: buf) - } - - func lower() -> UInt32 { switch self { {% for variant in e.variants() %} - case .{{ variant|enum_variant_swift }}: return {{ loop.index }} - {% endfor %} + {% if variant.has_fields() %} + case let .{{ variant.name()|enum_variant_swift }}({% for field in variant.fields() %}{{ field.name()|var_name_swift }}{%- if loop.last -%}{%- else -%},{%- endif -%}{% endfor %}): + buf.writeInt(Int32({{ loop.index }})) + {% for field in variant.fields() -%} + {{ field.name()|var_name_swift }}.write(into: buf) + {% endfor -%} + {% else %} + case .{{ variant.name()|enum_variant_swift }}: + buf.writeInt(Int32({{ loop.index }})) + {% endif %} + {%- endfor %} } } } diff --git a/uniffi_bindgen/src/bindings/swift/templates/RecordTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/RecordTemplate.swift index faa5e3ad28..d1ecf91098 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/RecordTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/RecordTemplate.swift @@ -5,16 +5,7 @@ public struct {{ rec.name()|class_name_swift }}: ViaFfiUsingByteBuffer, ViaFfi, // Default memberwise initializers are never public by default, so we // declare one manually. - public init( - {%- for field in rec.fields() %} - {{ field.name()|var_name_swift }}: {{ field.type_()|type_swift -}} - {%- match field.default_value() %} - {%- when Some with(literal) %} = {{ literal|literal_swift }} - {%- else %} - {%- endmatch -%} - {% if !loop.last %}, {% endif %} - {%- endfor %} - ) { + public init({% call swift::field_list_decl(rec) %}) { {%- for field in rec.fields() %} self.{{ field.name()|var_name_swift }} = {{ field.name()|var_name_swift }} {%- endfor %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/macros.swift b/uniffi_bindgen/src/bindings/swift/templates/macros.swift index f9133fd0e2..0a072ee9fb 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/macros.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/macros.swift @@ -55,6 +55,21 @@ {%- endfor %} {%- endmacro %} +{#- +// Field lists as used in Swift declarations of Records and Enums. +// Note the var_name_swift and type_swift filters. +-#} +{% macro field_list_decl(item) %} + {%- for field in item.fields() -%} + {{ field.name()|var_name_swift }}: {{ field.type_()|type_swift -}} + {%- match field.default_value() %} + {%- when Some with(literal) %} = {{ literal|literal_swift }} + {%- else %} + {%- endmatch -%} + {% if !loop.last %}, {% endif %} + {%- endfor %} +{%- endmacro %} + {% macro arg_list_protocol(func) %} {%- for arg in func.arguments() -%} diff --git a/uniffi_bindgen/src/interface/attributes.rs b/uniffi_bindgen/src/interface/attributes.rs index a386934612..31174ab5f3 100644 --- a/uniffi_bindgen/src/interface/attributes.rs +++ b/uniffi_bindgen/src/interface/attributes.rs @@ -14,7 +14,7 @@ //! all handled by a single abstraction. This might need to be refactored in future //! if we grow significantly more complicated attribute handling. -use std::convert::TryFrom; +use std::convert::{TryFrom, TryInto}; use anyhow::{bail, Result}; @@ -26,6 +26,7 @@ use anyhow::{bail, Result}; #[derive(Debug, Clone, Hash)] pub(super) enum Attribute { ByRef, + Enum, Error, Name(String), Threadsafe, @@ -36,6 +37,9 @@ impl Attribute { pub fn is_error(&self) -> bool { matches!(self, Attribute::Error) } + pub fn is_enum(&self) -> bool { + matches!(self, Attribute::Enum) + } } /// Convert a weedle `ExtendedAttribute` into an `Attribute` for a `ComponentInterface` member, @@ -49,6 +53,7 @@ impl TryFrom<&weedle::attribute::ExtendedAttribute<'_>> for Attribute { // Matches plain named attributes like "[ByRef"]. weedle::attribute::ExtendedAttribute::NoArgs(attr) => match (attr.0).0 { "ByRef" => Ok(Attribute::ByRef), + "Enum" => Ok(Attribute::Enum), "Error" => Ok(Attribute::Error), "Threadsafe" => Ok(Attribute::Threadsafe), _ => anyhow::bail!("ExtendedAttributeNoArgs not supported: {:?}", (attr.0).0), @@ -130,6 +135,16 @@ impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for EnumAttributes { } } +impl> TryFrom> for EnumAttributes { + type Error = anyhow::Error; + fn try_from(value: Option) -> Result { + match value { + None => Ok(Default::default()), + Some(v) => v.try_into(), + } + } +} + /// Represents UDL attributes that might appear on a function. /// /// This supports the `[Throws=ErrorName]` attribute for functions that @@ -161,6 +176,18 @@ impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for FunctionAttribut } } +impl> TryFrom> + for FunctionAttributes +{ + type Error = anyhow::Error; + fn try_from(value: Option) -> Result { + match value { + None => Ok(Default::default()), + Some(v) => v.try_into(), + } + } +} + /// Represents UDL attributes that might appear on a function argument. /// /// This supports the `[ByRef]` attribute for arguments that should be passed @@ -187,11 +214,27 @@ impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for ArgumentAttribut } } +impl> TryFrom> + for ArgumentAttributes +{ + type Error = anyhow::Error; + fn try_from(value: Option) -> Result { + match value { + None => Ok(Default::default()), + Some(v) => v.try_into(), + } + } +} + /// Represents UDL attributes that might appear on an `interface` definition. #[derive(Debug, Clone, Hash, Default)] pub(super) struct InterfaceAttributes(Vec); impl InterfaceAttributes { + pub fn contains_enum_attr(&self) -> bool { + self.0.iter().any(|attr| attr.is_enum()) + } + pub fn threadsafe(&self) -> bool { self.0 .iter() @@ -205,13 +248,30 @@ impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for InterfaceAttribu weedle_attributes: &weedle::attribute::ExtendedAttributeList<'_>, ) -> Result { let attrs = parse_attributes(weedle_attributes, |attr| match attr { + Attribute::Enum => Ok(()), Attribute::Threadsafe => Ok(()), - _ => bail!(format!("{:?} not supported for interface classes", attr)), + _ => bail!(format!("{:?} not supported for interface definition", attr)), })?; + // Can't be both `[Threadsafe]` and an `[Enum]`. + if attrs.len() > 1 { + bail!("conflicting attributes on interface definition"); + } Ok(Self(attrs)) } } +impl> TryFrom> + for InterfaceAttributes +{ + type Error = anyhow::Error; + fn try_from(value: Option) -> Result { + match value { + None => Ok(Default::default()), + Some(v) => v.try_into(), + } + } +} + // There may be some divergence between Methods and Functions at some point, // but not yet. pub(super) type MethodAttributes = FunctionAttributes; @@ -268,6 +328,15 @@ mod test { Ok(()) } + #[test] + fn test_enum() -> Result<()> { + let (_, node) = weedle::attribute::ExtendedAttribute::parse("Enum").unwrap(); + let attr = Attribute::try_from(&node)?; + assert!(matches!(attr, Attribute::Enum)); + assert!(attr.is_enum()); + Ok(()) + } + #[test] fn test_error() -> Result<()> { let (_, node) = weedle::attribute::ExtendedAttribute::parse("Error").unwrap(); @@ -300,6 +369,7 @@ mod test { assert!(matches!(attr, Attribute::Threadsafe)); Ok(()) } + #[test] fn test_throws() -> Result<()> { let (_, node) = weedle::attribute::ExtendedAttribute::parse("Throws=Name").unwrap(); @@ -440,12 +510,40 @@ mod test { Ok(()) } + #[test] + fn test_enum_attribute() -> Result<()> { + let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[Enum]").unwrap(); + let attrs = InterfaceAttributes::try_from(&node).unwrap(); + assert!(matches!(attrs.contains_enum_attr(), true)); + + let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[]").unwrap(); + let attrs = InterfaceAttributes::try_from(&node).unwrap(); + assert!(matches!(attrs.contains_enum_attr(), false)); + + let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[Threadsafe]").unwrap(); + let attrs = InterfaceAttributes::try_from(&node).unwrap(); + assert!(matches!(attrs.contains_enum_attr(), false)); + + let (_, node) = + weedle::attribute::ExtendedAttributeList::parse("[Threadsafe, Enum]").unwrap(); + let err = InterfaceAttributes::try_from(&node).unwrap_err(); + assert_eq!( + err.to_string(), + "conflicting attributes on interface definition" + ); + + Ok(()) + } + #[test] fn test_other_attributes_not_supported_for_interfaces() -> Result<()> { let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[Threadsafe, Error]").unwrap(); let err = InterfaceAttributes::try_from(&node).unwrap_err(); - assert_eq!(err.to_string(), "Error not supported for interface classes"); + assert_eq!( + err.to_string(), + "Error not supported for interface definition" + ); Ok(()) } } diff --git a/uniffi_bindgen/src/interface/callbacks.rs b/uniffi_bindgen/src/interface/callbacks.rs index af976f48a0..cf752b9808 100644 --- a/uniffi_bindgen/src/interface/callbacks.rs +++ b/uniffi_bindgen/src/interface/callbacks.rs @@ -110,7 +110,7 @@ impl APIConverter for weedle::CallbackInterfaceDefinition<'_> for member in &self.members.body { match member { weedle::interface::InterfaceMember::Operation(t) => { - let mut method = t.convert(ci)?; + let mut method: Method = t.convert(ci)?; method.object_name.push_str(object.name.as_str()); object.methods.push(method); } diff --git a/uniffi_bindgen/src/interface/enum_.rs b/uniffi_bindgen/src/interface/enum_.rs index 5d5f206b8a..0368654b6d 100644 --- a/uniffi_bindgen/src/interface/enum_.rs +++ b/uniffi_bindgen/src/interface/enum_.rs @@ -31,34 +31,86 @@ //! let e = ci.get_enum_definition("Example").unwrap(); //! assert_eq!(e.name(), "Example"); //! assert_eq!(e.variants().len(), 2); -//! assert_eq!(e.variants()[0], "one"); -//! assert_eq!(e.variants()[1], "two"); +//! assert_eq!(e.variants()[0].name(), "one"); +//! assert_eq!(e.variants()[1].name(), "two"); +//! # Ok::<(), anyhow::Error>(()) +//! ``` +//! +//! Like in Rust, UniFFI enums can contain associated data, but this needs to be +//! declared with a different syntax in order to work within the restrictions of +//! WebIDL. A declaration like this: +//! +//! ``` +//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" +//! # namespace example {}; +//! [Enum] +//! interface Example { +//! Zero(); +//! One(u32 first); +//! Two(u32 first, string second); +//! }; +//! # "##)?; +//! # Ok::<(), anyhow::Error>(()) +//! ``` +//! +//! Will result in an [`Enum`] member whose variants have associated fields: +//! +//! ``` +//! # let ci = uniffi_bindgen::interface::ComponentInterface::from_webidl(r##" +//! # namespace example {}; +//! # [Enum] +//! # interface ExampleWithData { +//! # Zero(); +//! # One(u32 first); +//! # Two(u32 first, string second); +//! # }; +//! # "##)?; +//! let e = ci.get_enum_definition("ExampleWithData").unwrap(); +//! assert_eq!(e.name(), "ExampleWithData"); +//! assert_eq!(e.variants().len(), 3); +//! assert_eq!(e.variants()[0].name(), "Zero"); +//! assert_eq!(e.variants()[0].fields().len(), 0); +//! assert_eq!(e.variants()[1].name(), "One"); +//! assert_eq!(e.variants()[1].fields().len(), 1); +//! assert_eq!(e.variants()[1].fields()[0].name(), "first"); //! # Ok::<(), anyhow::Error>(()) //! ``` -use anyhow::Result; +use anyhow::{bail, Result}; +use super::record::Field; +use super::types::Type; use super::{APIConverter, ComponentInterface}; -/// Represents a simple C-style enum, with named variants. +/// Represents an enum with named variants, each of which may have named +/// and typed fields. /// -/// In the FFI these are turned into a plain u32, with variants numbered -/// in the order they appear in the declaration, starting from 1. +/// Enums are passed across the FFI by serializing to a bytebuffer, with a +/// i32 indicating the variant followed by the serialization of each field. #[derive(Debug, Clone, Hash)] pub struct Enum { pub(super) name: String, - pub(super) variants: Vec, + pub(super) variants: Vec, + // "Flat" enums do not have, and will never have, variants with associated data. + pub(super) flat: bool, } impl Enum { pub fn name(&self) -> &str { &self.name } - pub fn variants(&self) -> Vec<&str> { - self.variants.iter().map(|v| v.as_str()).collect() + pub fn variants(&self) -> Vec<&Variant> { + self.variants.iter().collect() + } + + pub fn is_flat(&self) -> bool { + self.flat } } +// Note that we have two `APIConverter` impls here - one for the `enum` case +// and one for the `[Enum] interface` case. + impl APIConverter for weedle::EnumDefinition<'_> { fn convert(&self, _ci: &mut ComponentInterface) -> Result { Ok(Enum { @@ -68,14 +120,138 @@ impl APIConverter for weedle::EnumDefinition<'_> { .body .list .iter() - .map(|v| v.0.to_string()) - .collect(), + .map::, _>(|v| { + Ok(Variant { + name: v.0.to_string(), + ..Default::default() + }) + }) + .collect::>>()?, + flat: true, + }) + } +} + +impl APIConverter for weedle::InterfaceDefinition<'_> { + fn convert(&self, ci: &mut ComponentInterface) -> Result { + if self.inheritance.is_some() { + bail!("interface inheritence is not supported for enum interfaces"); + } + // We don't need to check `self.attributes` here; if calling code has dispatched + // to this impl then we already know there was an `[Enum]` attribute. + Ok(Enum { + name: self.identifier.0.to_string(), + variants: self + .members + .body + .iter() + .map::, _>(|member| match member { + weedle::interface::InterfaceMember::Operation(t) => Ok(t.convert(ci)?), + _ => bail!( + "interface member type {:?} not supported in enum interface", + member + ), + }) + .collect::>>()?, + flat: false, + }) + } +} + +/// Represents an individual variant in an Enum. +/// +/// Each variant has a name and zero or more fields. +#[derive(Debug, Clone, Default, Hash)] +pub struct Variant { + pub(super) name: String, + pub(super) fields: Vec, +} + +impl Variant { + pub fn name(&self) -> &str { + &self.name + } + pub fn fields(&self) -> Vec<&Field> { + self.fields.iter().collect() + } + + pub fn has_fields(&self) -> bool { + self.fields.len() > 0 + } +} + +impl APIConverter for weedle::interface::OperationInterfaceMember<'_> { + fn convert(&self, ci: &mut ComponentInterface) -> Result { + if self.special.is_some() { + bail!("special operations not supported"); + } + if let Some(weedle::interface::StringifierOrStatic::Stringifier(_)) = self.modifier { + bail!("stringifiers are not supported"); + } + // OK, so this is a little weird. + // The syntax we use for enum interface members is `Name(type arg, ...);`, which parses + // as an anonymous operation where `Name` is the return type. We re-interpret it to + // use `Name` as the name of the variant. + if self.identifier.is_some() { + bail!("enum interface members must not have a method name"); + } + let name: String = { + use weedle::types::{ + NonAnyType::Identifier, ReturnType, SingleType::NonAny, Type::Single, + }; + match &self.return_type { + ReturnType::Type(Single(NonAny(Identifier(id)))) => id.type_.0.to_owned(), + _ => bail!("enum interface members must have plain identifers as names"), + } + }; + Ok(Variant { + name, + fields: self + .args + .body + .list + .iter() + .map(|arg| arg.convert(ci)) + .collect::>>()?, + }) + } +} + +impl APIConverter for weedle::argument::Argument<'_> { + fn convert(&self, ci: &mut ComponentInterface) -> Result { + match self { + weedle::argument::Argument::Single(t) => t.convert(ci), + weedle::argument::Argument::Variadic(_) => bail!("variadic arguments not supported"), + } + } +} + +impl APIConverter for weedle::argument::SingleArgument<'_> { + fn convert(&self, ci: &mut ComponentInterface) -> Result { + let type_ = ci.resolve_type_expression(&self.type_)?; + if let Type::Object(_) = type_ { + bail!("Objects cannot currently be used in enum variant data"); + } + if self.default.is_some() { + bail!("enum interface variant fields must not have default values"); + } + if self.attributes.is_some() { + bail!("enum interface variant fields must not have attributes"); + } + // TODO: maybe we should use our own `Field` type here with just name and type, + // rather than appropriating record::Field..? + Ok(Field { + name: self.identifier.0.to_string(), + type_, + required: false, + default: None, }) } } #[cfg(test)] mod test { + use super::super::ffi::FFIType; use super::*; #[test] @@ -94,4 +270,131 @@ mod test { ); Ok(()) } + + #[test] + fn test_associated_data() -> Result<()> { + const UDL: &str = r##" + namespace test { + void takes_an_enum(TestEnum e); + void takes_an_enum_with_data(TestEnumWithData ed); + TestEnum returns_an_enum(); + TestEnumWithData returns_an_enum_with_data(); + }; + + enum TestEnum { "one", "two" }; + + [Enum] + interface TestEnumWithData { + Zero(); + One(u32 first); + Two(u32 first, string second); + }; + + [Enum] + interface TestEnumWithoutData { + One(); + Two(); + }; + "##; + let ci = ComponentInterface::from_webidl(UDL).unwrap(); + assert_eq!(ci.iter_enum_definitions().len(), 3); + assert_eq!(ci.iter_function_definitions().len(), 4); + + // The "flat" enum with no associated data. + let e = ci.get_enum_definition("TestEnum").unwrap(); + assert!(e.is_flat()); + assert_eq!(e.variants().len(), 2); + assert_eq!( + e.variants().iter().map(|v| v.name()).collect::>(), + vec!["one", "two"] + ); + assert_eq!(e.variants()[0].fields().len(), 0); + assert_eq!(e.variants()[1].fields().len(), 0); + + // The enum with associated data. + let ed = ci.get_enum_definition("TestEnumWithData").unwrap(); + assert!(!ed.is_flat()); + assert_eq!(ed.variants().len(), 3); + assert_eq!( + ed.variants().iter().map(|v| v.name()).collect::>(), + vec!["Zero", "One", "Two"] + ); + assert_eq!(ed.variants()[0].fields().len(), 0); + assert_eq!( + ed.variants()[1] + .fields() + .iter() + .map(|f| f.name()) + .collect::>(), + vec!["first"] + ); + assert_eq!( + ed.variants()[1] + .fields() + .iter() + .map(|f| f.type_()) + .collect::>(), + vec![Type::UInt32] + ); + assert_eq!( + ed.variants()[2] + .fields() + .iter() + .map(|f| f.name()) + .collect::>(), + vec!["first", "second"] + ); + assert_eq!( + ed.variants()[2] + .fields() + .iter() + .map(|f| f.type_()) + .collect::>(), + vec![Type::UInt32, Type::String] + ); + + // The enum declared via interface, but with no associated data. + let ewd = ci.get_enum_definition("TestEnumWithoutData").unwrap(); + assert!(!ewd.is_flat()); + assert_eq!(ewd.variants().len(), 2); + assert_eq!( + ewd.variants().iter().map(|v| v.name()).collect::>(), + vec!["One", "Two"] + ); + assert_eq!(ewd.variants()[0].fields().len(), 0); + assert_eq!(ewd.variants()[1].fields().len(), 0); + + // Flat enums pass over the FFI as bytebuffers. + // (It might be nice to optimize these to pass as plain integers, but that's + // difficult atop the current factoring of `ComponentInterface` and friends). + let farg = ci.get_function_definition("takes_an_enum").unwrap(); + assert_eq!(farg.arguments()[0].type_(), Type::Enum("TestEnum".into())); + assert_eq!(farg.ffi_func().arguments()[0].type_(), FFIType::RustBuffer); + let fret = ci.get_function_definition("returns_an_enum").unwrap(); + assert!(matches!(fret.return_type(), Some(Type::Enum(nm)) if nm == "TestEnum")); + assert!(matches!( + fret.ffi_func().return_type(), + Some(FFIType::RustBuffer) + )); + + // Enums with associated data pass over the FFI as bytebuffers. + let farg = ci + .get_function_definition("takes_an_enum_with_data") + .unwrap(); + assert_eq!( + farg.arguments()[0].type_(), + Type::Enum("TestEnumWithData".into()) + ); + assert_eq!(farg.ffi_func().arguments()[0].type_(), FFIType::RustBuffer); + let fret = ci + .get_function_definition("returns_an_enum_with_data") + .unwrap(); + assert!(matches!(fret.return_type(), Some(Type::Enum(nm)) if nm == "TestEnumWithData")); + assert!(matches!( + fret.ffi_func().return_type(), + Some(FFIType::RustBuffer) + )); + + Ok(()) + } } diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index f84e7ff63a..24b4e24380 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -18,7 +18,7 @@ /// For the types that involve memory allocation, we make a distinction between /// "owned" types (the recipient must free it, or pass it to someone else) and /// "borrowed" types (the sender must keep it alive for the duration of the call). -#[derive(Debug, Clone, Hash)] +#[derive(Debug, Clone, Hash, PartialEq, Eq)] pub enum FFIType { // N.B. there are no booleans at this layer, since they cause problems for JNA. UInt8, diff --git a/uniffi_bindgen/src/interface/function.rs b/uniffi_bindgen/src/interface/function.rs index 6cf9a67e1b..39ded3204d 100644 --- a/uniffi_bindgen/src/interface/function.rs +++ b/uniffi_bindgen/src/interface/function.rs @@ -123,10 +123,7 @@ impl APIConverter for weedle::namespace::OperationNamespaceMember<'_> return_type, arguments: self.args.body.list.convert(ci)?, ffi_func: Default::default(), - attributes: match &self.attributes { - Some(attr) => FunctionAttributes::try_from(attr)?, - None => Default::default(), - }, + attributes: FunctionAttributes::try_from(self.attributes.as_ref())?, }) } } @@ -186,10 +183,7 @@ impl APIConverter for weedle::argument::SingleArgument<'_> { None => None, Some(v) => Some(convert_default_value(&v.value, &type_)?), }; - let by_ref = match &self.attributes { - Some(attrs) => ArgumentAttributes::try_from(attrs)?.by_ref(), - None => false, - }; + let by_ref = ArgumentAttributes::try_from(self.attributes.as_ref())?.by_ref(); Ok(Argument { name: self.identifier.0.to_string(), type_, diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index 2195fdd047..aa26058e82 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -113,9 +113,7 @@ impl<'ci> ComponentInterface { ci.types.add_type_definitions_from(defns.as_slice())?; // With those names resolved, we can build a complete representation of the API. APIBuilder::process(&defns, &mut ci)?; - if ci.namespace.is_empty() { - bail!("missing namespace definition"); - } + ci.check_consistency()?; // Now that the high-level API is settled, we can derive the low-level FFI. ci.derive_ffi_funcs()?; Ok(ci) @@ -423,8 +421,13 @@ impl<'ci> ComponentInterface { /// Called by `APIBuilder` impls to add a newly-parsed function definition to the `ComponentInterface`. fn add_function_definition(&mut self, defn: Function) -> Result<()> { + // Since functions are not a first-class type, we have to check for duplicates here + // rather than relying on the type-finding pass to catch them. if self.functions.iter().any(|f| f.name == defn.name) { - bail!("duplicate function definition: {}", defn.name); + bail!("duplicate function definition: \"{}\"", defn.name); + } + if !matches!(self.types.get_type_definition(defn.name()), None) { + bail!("Conflicting type definition for \"{}\"", defn.name()); } self.functions.push(defn); Ok(()) @@ -451,6 +454,29 @@ impl<'ci> ComponentInterface { Ok(()) } + /// Perform global consistency checks on the declared interface. + /// + /// This method checks for consistency problems in the declared interface + /// as a whole, and which can only be detected after we've finished defining + /// the entire interface. + fn check_consistency(&self) -> Result<()> { + if self.namespace.is_empty() { + bail!("missing namespace definition"); + } + // To keep codegen tractable, enum variant names must not shadow type names. + for e in self.enums.iter() { + for variant in e.variants.iter() { + if self.types.get_type_definition(variant.name()).is_some() { + bail!( + "Enum variant names must not shadow type names: \"{}\"", + variant.name() + ) + } + } + } + Ok(()) + } + /// Automatically derive the low-level FFI functions from the high-level types in the interface. /// /// This should only be called after the high-level types have been completed defined, otherwise @@ -524,12 +550,8 @@ impl APIBuilder for weedle::Definition<'_> { weedle::Definition::Namespace(d) => d.process(ci), weedle::Definition::Enum(d) => { // We check if the enum represents an error... - let is_error = if let Some(attrs) = &d.attributes { - attributes::EnumAttributes::try_from(attrs)?.contains_error_attr() - } else { - false - }; - if is_error { + let attrs = attributes::EnumAttributes::try_from(d.attributes.as_ref())?; + if attrs.contains_error_attr() { let err = d.convert(ci)?; ci.add_error_definition(err) } else { @@ -542,8 +564,14 @@ impl APIBuilder for weedle::Definition<'_> { ci.add_record_definition(rec) } weedle::Definition::Interface(d) => { - let obj = d.convert(ci)?; - ci.add_object_definition(obj) + let attrs = attributes::InterfaceAttributes::try_from(d.attributes.as_ref())?; + if attrs.contains_enum_attr() { + let e = d.convert(ci)?; + ci.add_enum_definition(e) + } else { + let obj = d.convert(ci)?; + ci.add_object_definition(obj) + } } weedle::Definition::CallbackInterface(d) => { let obj = d.convert(ci)?; @@ -631,4 +659,73 @@ mod test { assert_ne!(ci1.checksum(), ci2.checksum()); } } + + #[test] + fn test_duplicate_type_names_are_an_error() { + const UDL: &str = r#" + namespace test{}; + interface Testing { + constructor(); + }; + dictionary Testing { + u32 field; + }; + "#; + let err = ComponentInterface::from_webidl(UDL).unwrap_err(); + assert_eq!( + err.to_string(), + "Conflicting type definition for \"Testing\"" + ); + + const UDL2: &str = r#" + namespace test{}; + enum Testing { + "one", "two" + }; + [Error] + enum Testing { "three", "four" }; + "#; + let err = ComponentInterface::from_webidl(UDL2).unwrap_err(); + assert_eq!( + err.to_string(), + "Conflicting type definition for \"Testing\"" + ); + + const UDL3: &str = r#" + namespace test{ + u32 Testing(); + }; + enum Testing { + "one", "two" + }; + "#; + let err = ComponentInterface::from_webidl(UDL3).unwrap_err(); + assert_eq!( + err.to_string(), + "Conflicting type definition for \"Testing\"" + ); + } + + #[test] + fn test_enum_variant_names_dont_shadow_types() { + // There are some edge-cases during codegen where we don't know how to disambiguate + // between an enum variant reference and a top-level type reference, so we + // disallow it in order to give a more scrutable error to the consumer. + const UDL: &str = r#" + namespace test{}; + interface Testing { + constructor(); + }; + [Enum] + interface HardToCodegenFor { + Testing(); + OtherVariant(u32 field); + }; + "#; + let err = ComponentInterface::from_webidl(UDL).unwrap_err(); + assert_eq!( + err.to_string(), + "Enum variant names must not shadow type names: \"Testing\"" + ); + } } diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index 1b385691c6..547de35b0e 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -171,14 +171,14 @@ impl APIConverter for weedle::InterfaceDefinition<'_> { for member in &self.members.body { match member { weedle::interface::InterfaceMember::Constructor(t) => { - let cons = t.convert(ci)?; + let cons: Constructor = t.convert(ci)?; if !member_names.insert(cons.name.clone()) { bail!("Duplicate interface member name: \"{}\"", cons.name()) } object.constructors.push(cons); } weedle::interface::InterfaceMember::Operation(t) => { - let mut method = t.convert(ci)?; + let mut method: Method = t.convert(ci)?; if !member_names.insert(method.name.clone()) { bail!("Duplicate interface member name: \"{}\"", method.name()) } @@ -388,10 +388,7 @@ impl APIConverter for weedle::interface::OperationInterfaceMember<'_> { arguments: self.args.body.list.convert(ci)?, return_type, ffi_func: Default::default(), - attributes: match &self.attributes { - Some(attr) => MethodAttributes::try_from(attr)?, - None => Default::default(), - }, + attributes: MethodAttributes::try_from(self.attributes.as_ref())?, }) } } diff --git a/uniffi_bindgen/src/interface/record.rs b/uniffi_bindgen/src/interface/record.rs index 806c7423ad..6072f65f35 100644 --- a/uniffi_bindgen/src/interface/record.rs +++ b/uniffi_bindgen/src/interface/record.rs @@ -88,10 +88,10 @@ impl APIConverter for weedle::DictionaryDefinition<'_> { // Represents an individual field on a Record. #[derive(Debug, Clone, Hash)] pub struct Field { - name: String, - type_: Type, - required: bool, - default: Option, + pub(super) name: String, + pub(super) type_: Type, + pub(super) required: bool, + pub(super) default: Option, } impl Field { diff --git a/uniffi_bindgen/src/interface/types/finder.rs b/uniffi_bindgen/src/interface/types/finder.rs index ede450e058..97aab06a20 100644 --- a/uniffi_bindgen/src/interface/types/finder.rs +++ b/uniffi_bindgen/src/interface/types/finder.rs @@ -21,7 +21,7 @@ use std::convert::TryFrom; use anyhow::{bail, Result}; -use super::super::attributes::EnumAttributes; +use super::super::attributes::{EnumAttributes, InterfaceAttributes}; use super::{Type, TypeUniverse}; /// Trait to help with an early "type discovery" phase when processing the UDL. @@ -58,7 +58,12 @@ impl TypeFinder for weedle::Definition<'_> { impl TypeFinder for weedle::InterfaceDefinition<'_> { fn add_type_definitions_to(&self, types: &mut TypeUniverse) -> Result<()> { let name = self.identifier.0.to_string(); - types.add_type_definition(self.identifier.0, Type::Object(name)) + // Some enum types are defined using an `interface` with a special attribute. + if InterfaceAttributes::try_from(self.attributes.as_ref())?.contains_enum_attr() { + types.add_type_definition(self.identifier.0, Type::Enum(name)) + } else { + types.add_type_definition(self.identifier.0, Type::Object(name)) + } } } @@ -73,13 +78,11 @@ impl TypeFinder for weedle::EnumDefinition<'_> { fn add_type_definitions_to(&self, types: &mut TypeUniverse) -> Result<()> { let name = self.identifier.0.to_string(); // Our error types are defined using an `enum` with a special attribute. - if let Some(attrs) = &self.attributes { - let attrs = EnumAttributes::try_from(attrs)?; - if attrs.contains_error_attr() { - return types.add_type_definition(self.identifier.0, Type::Error(name)); - } + if EnumAttributes::try_from(self.attributes.as_ref())?.contains_error_attr() { + types.add_type_definition(self.identifier.0, Type::Error(name)) + } else { + types.add_type_definition(self.identifier.0, Type::Enum(name)) } - types.add_type_definition(self.identifier.0, Type::Enum(name)) } } diff --git a/uniffi_bindgen/src/interface/types/mod.rs b/uniffi_bindgen/src/interface/types/mod.rs index cab0369175..c4b009460f 100644 --- a/uniffi_bindgen/src/interface/types/mod.rs +++ b/uniffi_bindgen/src/interface/types/mod.rs @@ -133,14 +133,14 @@ impl Into for &Type { Type::Object(_) => FFIType::UInt64, // Callback interfaces are passed as opaque integer handles. Type::CallbackInterface(_) => FFIType::UInt64, - // Enums are passed as integers. - Type::Enum(_) => FFIType::UInt32, // Errors have their own special type. Type::Error(_) => FFIType::RustError, // Other types are serialized into a bytebuffer and deserialized on the other side. - Type::Record(_) | Type::Optional(_) | Type::Sequence(_) | Type::Map(_) => { - FFIType::RustBuffer - } + Type::Enum(_) + | Type::Record(_) + | Type::Optional(_) + | Type::Sequence(_) + | Type::Map(_) => FFIType::RustBuffer, } } } @@ -184,7 +184,7 @@ impl TypeUniverse { } let type_ = self.add_known_type(type_)?; match self.type_definitions.entry(name.to_string()) { - Entry::Occupied(_) => bail!("Conflicting type definition for {}", name), + Entry::Occupied(_) => bail!("Conflicting type definition for \"{}\"", name), Entry::Vacant(e) => { e.insert(type_); Ok(()) @@ -193,7 +193,7 @@ impl TypeUniverse { } /// Get the [Type] corresponding to a given name, if any. - fn get_type_definition(&self, name: &str) -> Option { + pub(super) fn get_type_definition(&self, name: &str) -> Option { self.type_definitions.get(name).cloned() } diff --git a/uniffi_bindgen/src/templates/EnumTemplate.rs b/uniffi_bindgen/src/templates/EnumTemplate.rs index 3f09a40fbf..7c2d74df46 100644 --- a/uniffi_bindgen/src/templates/EnumTemplate.rs +++ b/uniffi_bindgen/src/templates/EnumTemplate.rs @@ -1,45 +1,44 @@ {# -// For each enum declared in the UDL, we assume the caller as provided a corresponding +// For each enum declared in the UDL, we assume the caller has provided a corresponding // rust `enum`. We provide the traits for sending it across the FFI, which will fail to // compile if the provided struct has a different shape to the one declared in the UDL. -// -// The enum will be sent over the FFI as a u32, with values assigned according to the -// order of items *as declared in the UDL file*. This might be different to the order -// of items as declared in the rust code, but no harm will come from it. #} #[doc(hidden)] unsafe impl uniffi::ViaFfi for {{ e.name() }} { - type FfiType = u32; + type FfiType = uniffi::RustBuffer; fn lower(self) -> Self::FfiType { - match self { - // If the provided enum doesn't match the options defined in the UDL then - // this match will fail to compile, with a type error to guide the way. - {%- for variant in e.variants() %} - {{ e.name() }}::{{ variant }} => {{ loop.index }}, - {%- endfor %} - } + uniffi::lower_into_buffer(self) } fn try_lift(v: Self::FfiType) -> uniffi::deps::anyhow::Result { - Ok(match v { - {%- for variant in e.variants() %} - {{ loop.index }} => {{ e.name() }}::{{ variant }}, - {%- endfor %} - _ => uniffi::deps::anyhow::bail!("Invalid {{ e.name() }} enum value: {}", v), - }) + uniffi::try_lift_from_buffer(v) } fn write(&self, buf: &mut B) { - buf.put_u32(match self { + match self { {%- for variant in e.variants() %} - {{ e.name() }}::{{ variant }} => {{ loop.index }}, + {{ e.name() }}::{{ variant.name() }} { {% for field in variant.fields() %}{{ field.name() }}, {%- endfor %} } => { + buf.put_i32({{ loop.index }}); + {% for field in variant.fields() -%} + <{{ field.type_()|type_rs }} as uniffi::ViaFfi>::write({{ field.name() }}, buf); + {%- endfor %} + }, {%- endfor %} - }); + }; } fn try_read(buf: &mut B) -> uniffi::deps::anyhow::Result { uniffi::check_remaining(buf, 4)?; - ::try_lift(buf.get_u32()) + Ok(match buf.get_i32() { + {%- for variant in e.variants() %} + {{ loop.index }} => {{ e.name() }}::{{ variant.name() }}{% if variant.has_fields() %} { + {% for field in variant.fields() %} + {{ field.name() }}: <{{ field.type_()|type_rs }} as uniffi::ViaFfi>::try_read(buf)?, + {%- endfor %} + }{% endif %}, + {%- endfor %} + v @ _ => uniffi::deps::anyhow::bail!("Invalid {{ e.name() }} enum value: {}", v), + }) } }