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), + }) } }