From 3a63c2bce8efa8d64d4644f1b4dc699c4ff55dae 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 | 10 +- .../tests/bindings/test_rondpoint.py | 20 +- .../tests/bindings/test_rondpoint.swift | 10 +- .../gecko_js/templates/SharedHeaderTemplate.h | 34 +- .../gecko_js/templates/WebIDLTemplate.webidl | 6 +- .../src/bindings/gecko_js/webidl.rs | 2 +- .../bindings/kotlin/templates/EnumTemplate.kt | 93 +++++- .../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 | 38 ++- uniffi_bindgen/src/interface/attributes.rs | 104 +++++- uniffi_bindgen/src/interface/callbacks.rs | 2 +- uniffi_bindgen/src/interface/enum_.rs | 302 +++++++++++++++++- 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 ++- 30 files changed, 871 insertions(+), 158 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..172f9a7dde 100644 --- a/examples/rondpoint/tests/bindings/test_rondpoint.kts +++ b/examples/rondpoint/tests/bindings/test_rondpoint.kts @@ -6,7 +6,15 @@ 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") +)) assert(switcheroo(false)) diff --git a/examples/rondpoint/tests/bindings/test_rondpoint.py b/examples/rondpoint/tests/bindings/test_rondpoint.py index 86abb17b09..1cdcc14de0 100644 --- a/examples/rondpoint/tests/bindings/test_rondpoint.py +++ b/examples/rondpoint/tests/bindings/test_rondpoint.py @@ -8,7 +8,15 @@ 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 @@ -19,9 +27,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 +95,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..afc0943ffc 100644 --- a/examples/rondpoint/tests/bindings/test_rondpoint.swift +++ b/examples/rondpoint/tests/bindings/test_rondpoint.swift @@ -6,7 +6,15 @@ 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(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..b5a51dcca0 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.has_associated_data() %} +MOZ_STATIC_ASSERT(false, "Sorry the gecko-js backend does not yet support enums with associated data"); +{% 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..85906ecc5b 100644 --- a/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl +++ b/uniffi_bindgen/src/bindings/gecko_js/templates/WebIDLTemplate.webidl @@ -14,11 +14,15 @@ dictionary {{ rec.name()|class_name_webidl(context) }} { {% endfor %} {%- for e in ci.iter_enum_definitions() %} +{% if e.has_associated_data() %} +MOZ_STATIC_ASSERT(false, "Sorry the gecko-js backend does not yet support enums with associated data"); +{% else %} 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 %} }; +{% endif %} {% endfor %} {%- let functions = ci.iter_function_definitions() %} diff --git a/uniffi_bindgen/src/bindings/gecko_js/webidl.rs b/uniffi_bindgen/src/bindings/gecko_js/webidl.rs index 418511b4c5..3d13572dc8 100644 --- a/uniffi_bindgen/src/bindings/gecko_js/webidl.rs +++ b/uniffi_bindgen/src/bindings/gecko_js/webidl.rs @@ -69,7 +69,7 @@ impl WebIDLType { /// the C++ implementation; `false` if by value. pub fn needs_out_param(&self) -> bool { match self { - WebIDLType::Flat(Type::String) | WebIDLType::Flat(Type::Record(_)) => true, + WebIDLType::Flat(Type::String) | WebIDLType::Flat(Type::Record(_)) | WebIDLType::Flat(Type::Enum(_)) => true, WebIDLType::Map(_) | WebIDLType::Sequence(_) => true, WebIDLType::Optional(inner) | WebIDLType::OptionalWithDefaultValue(inner) diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt index 2a9e59d6d4..78ddf7e3c9 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt @@ -1,19 +1,94 @@ -enum class {{ e.name()|class_name_kt }} { - {% for variant in e.variants() %} - {{ variant|enum_variant_kt }}{% if loop.last %};{% else %},{% endif %} +{# +// 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.has_associated_data() %} + +sealed class {{ e.name()|class_name_kt }} { + {% for variant in e.variants() -%} + class {{ variant.name()|class_name_kt }}({% if variant.has_fields() %} + {% for field in variant.fields() -%} + val {{ field.name()|var_name_kt }}: {{ field.type_()|type_kt}}{% if loop.last %}{% else %}, {% endif %} + {% endfor -%} + {%- endif %}) : {{ e.name()|class_name_kt }}() { + override fun write(buf: RustBufferBuilder) { + buf.putInt({{ loop.index }}) + {% for field in variant.fields() -%} + {{ "(this.{})"|format(field.name())|write_kt("buf", field.type_()) }} + {% endfor -%} + } + override fun equals(other: Any?) : Boolean = + if (other is {{ e.name()|class_name_kt }}.{{ variant.name()|class_name_kt }}) { + {% if variant.has_fields() -%} + {% for field in variant.fields() -%} + {{ field.name()|var_name_kt }} == other.{{ field.name()|var_name_kt }}{% if loop.last %}{% else %} && {% endif -%} + {% endfor -%} + {% else -%} + true + {%- endif %} + } else { + false + } + } {% 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): {{ 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(): RustBuffer.ByValue { + return lowerIntoRustBuffer(this, {v, buf -> v.write(buf)}) + } + + internal open fun write(buf: RustBufferBuilder) { + throw RuntimeException("enum variant should have overridden `write` method, something is very wrong!!") + } +} + +{% else %} + +enum class {{ e.name()|class_name_kt }} { + {% for variant in e.variants() -%} + {{ variant.name()|enum_variant_kt }}{% if loop.last %};{% else %},{% 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) = + try { values()[buf.getInt() - 1] } catch (e: IndexOutOfBoundsException) { throw RuntimeException("invalid enum value, something is very wrong!!", e) } - - internal fun read(buf: ByteBuffer) = lift(buf.getInt()) } - 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) { + buf.putInt(this.ordinal + 1) + } } + +{% endif %} diff --git a/uniffi_bindgen/src/bindings/python/gen_python.rs b/uniffi_bindgen/src/bindings/python/gen_python.rs index bc06e6fb84..b2ee5962b7 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python.rs @@ -127,9 +127,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!( @@ -155,11 +156,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 @@ -180,11 +184,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..99cf1837dc 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.has_associated_data() %} + +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 by a subclass of the main +# enum subclass, so that method calls and instance checks etc will work intuitively. +# We might be able to do this a little more nearly 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 %} + +{% else %} + 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 %} + +{% 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..8e3590501b 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.has_associated_data() %} 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..e687bdf9a4 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.has_associated_data() -%} + 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..4e13b1f852 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/EnumTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/EnumTemplate.swift @@ -1,30 +1,40 @@ -public enum {{ e.name()|class_name_swift }}: ViaFfi { +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 %}( + {% for field in variant.fields() %} + {{ field.name()|var_name_swift }}: {{ field.type_()|type_swift }}{%- if loop.last -%}{%- else -%},{%- endif -%} + {% endfor %} + ){% 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/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..c8b1f54718 100644 --- a/uniffi_bindgen/src/interface/enum_.rs +++ b/uniffi_bindgen/src/interface/enum_.rs @@ -31,34 +31,84 @@ //! 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, } 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 has_associated_data(&self) -> bool { + self.variants.iter().any(Variant::has_fields) } } +// 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 +118,136 @@ 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::>>()?, + }) + } +} + +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::>>()?, + }) + } +} + +/// 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 +266,112 @@ 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); + }; + "#; + let ci = ComponentInterface::from_webidl(UDL).unwrap(); + assert_eq!(ci.iter_enum_definitions().len(), 2); + assert_eq!(ci.iter_function_definitions().len(), 4); + + // The "plain old enum" with no associated data. + let e = ci.get_enum_definition("TestEnum").unwrap(); + assert!(!e.has_associated_data()); + 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.has_associated_data()); + 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] + ); + + // Enums with no associated data 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), + }) } }