Skip to content

Commit

Permalink
Add support for non-exhaustive enums (mozilla#1593)
Browse files Browse the repository at this point in the history
UDL types can now have the `[NonExhaustive]` attribute.  If present, we
add a default branch arm to make things compile correctly with
non-exhaustive enums.  I didn't want to make this the default behavior,
since in the normal case we want missing variants to result in compile
errors.

In theory, this also adds the `non_exhaustive` attribute for
proc-macros.  However, there's no way to test this because
`non_exhaustive` only matters for types defined in remote crates and
there's currently no proc-macro support for those.

One slightly silly part of this is the flow of metadata.  It doesn't
make much sense as a metadata field, since it doesn't affect bindings
generation at all.  However, it needs to be because of the way that data
flows from `uniffi_udl` -> `uniffi_meta` -> `uniffi_bindgen` ->
`uniffi_macros`.
  • Loading branch information
bendk committed Jan 3, 2024
1 parent fb8dd5c commit 2272b56
Show file tree
Hide file tree
Showing 16 changed files with 266 additions and 50 deletions.
23 changes: 23 additions & 0 deletions docs/manual/src/udl/enumerations.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ enum Animal {
};
```

## Enums with fields

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:
Expand All @@ -40,3 +42,24 @@ interface IpAddr {
```

Only enums with named fields are supported by this syntax.

## Remote, non-exhaustive enums

One corner case is an enum that's:
- Defined in another crate.
- Has the [non_exhaustive` attribute](https://doc.rust-lang.org/reference/attributes/type_system.html#the-non_exhaustive-attribute).

In this case, UniFFI needs to generate a default arm when matching against the enum variants, or else a compile error will be generated.
Use the `[NonExhaustive]` attribute to handle this case:

```idl
[Enum]
[NonExhaustive]
interface Message {
Send(u32 from, u32 to, string contents);
Quit();
};
```

**Note:** since UniFFI generates a default arm, if you leave out a variant, or if the upstream crate adds a new variant, this won't be caught at compile time.
Any attempt to pass that variant across the FFI will result in a panic.
6 changes: 6 additions & 0 deletions fixtures/ext-types/external-crate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ pub struct ExternalCrateInterface {
pub sval: String,
}

#[non_exhaustive]
pub enum ExternalCrateNonExhaustiveEnum {
One,
Two,
}

// This type is referenced as an "external" type as an interface.
impl ExternalCrateInterface {
pub fn new(sval: String) -> Self {
Expand Down
7 changes: 7 additions & 0 deletions fixtures/ext-types/lib/src/ext-types-lib.udl
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ interface ExternalCrateInterface {
string value();
};

[NonExhaustive]
enum ExternalCrateNonExhaustiveEnum {
"One",
"Two",
};

// And a new type here to tie them all together.
dictionary CombinedType {
UniffiOneEnum uoe;
Expand All @@ -86,4 +92,5 @@ dictionary CombinedType {
Handle? maybe_handle;

ExternalCrateDictionary ecd;
ExternalCrateNonExhaustiveEnum ecnee;
};
6 changes: 5 additions & 1 deletion fixtures/ext-types/lib/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use custom_types::Handle;
use ext_types_external_crate::{ExternalCrateDictionary, ExternalCrateInterface};
use ext_types_external_crate::{
ExternalCrateDictionary, ExternalCrateInterface, ExternalCrateNonExhaustiveEnum,
};
use ext_types_guid::Guid;
use std::sync::Arc;
use uniffi_one::{
Expand Down Expand Up @@ -28,6 +30,7 @@ pub struct CombinedType {
pub maybe_handle: Option<Handle>,

pub ecd: ExternalCrateDictionary,
pub ecnee: ExternalCrateNonExhaustiveEnum,
}

fn get_combined_type(existing: Option<CombinedType>) -> CombinedType {
Expand Down Expand Up @@ -59,6 +62,7 @@ fn get_combined_type(existing: Option<CombinedType>) -> CombinedType {
maybe_handle: Some(Handle(4)),

ecd: ExternalCrateDictionary { sval: "ecd".into() },
ecnee: ExternalCrateNonExhaustiveEnum::One,
})
}

Expand Down
5 changes: 5 additions & 0 deletions fixtures/metadata/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ mod test_metadata {
docstring: None,
},
],
non_exhaustive: false,
docstring: None,
},
);
Expand Down Expand Up @@ -266,6 +267,7 @@ mod test_metadata {
docstring: None,
},
],
non_exhaustive: false,
docstring: None,
},
);
Expand Down Expand Up @@ -298,6 +300,7 @@ mod test_metadata {
docstring: None,
},
],
non_exhaustive: false,
docstring: None,
},
);
Expand Down Expand Up @@ -325,6 +328,7 @@ mod test_metadata {
docstring: None,
},
],
non_exhaustive: false,
docstring: None,
},
is_flat: true,
Expand Down Expand Up @@ -373,6 +377,7 @@ mod test_metadata {
docstring: None,
},
],
non_exhaustive: false,
docstring: None,
},
is_flat: false,
Expand Down
7 changes: 7 additions & 0 deletions uniffi_bindgen/src/interface/enum_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ pub struct Enum {
// * For an Enum not used as an error but which has no variants with data, `flat` will be
// false when generating the scaffolding but `true` when generating bindings.
pub(super) flat: bool,
pub(super) non_exhaustive: bool,
#[checksum_ignore]
pub(super) docstring: Option<String>,
}
Expand Down Expand Up @@ -233,6 +234,10 @@ impl Enum {
self.flat
}

pub fn is_non_exhaustive(&self) -> bool {
self.non_exhaustive
}

pub fn iter_types(&self) -> TypeIterator<'_> {
Box::new(self.variants.iter().flat_map(Variant::iter_types))
}
Expand All @@ -257,6 +262,7 @@ impl Enum {
.map(TryInto::try_into)
.collect::<Result<_>>()?,
flat,
non_exhaustive: meta.non_exhaustive,
docstring: meta.docstring.clone(),
})
}
Expand Down Expand Up @@ -635,6 +641,7 @@ mod test {
name: "test".to_string(),
variants: vec![],
flat: false,
non_exhaustive: false,
docstring: None,
};

Expand Down
6 changes: 5 additions & 1 deletion uniffi_bindgen/src/scaffolding/templates/EnumTemplate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
// public so other crates can refer to it via an `[External='crate'] typedef`
#}
#[::uniffi::derive_enum_for_udl]
#[::uniffi::derive_enum_for_udl(
{%- if e.is_non_exhaustive() -%}
non_exhaustive,
{%- endif %}
)]
enum r#{{ e.name() }} {
{%- for variant in e.variants() %}
r#{{ variant.name() }} {
Expand Down
3 changes: 3 additions & 0 deletions uniffi_bindgen/src/scaffolding/templates/ErrorTemplate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
with_try_read,
{%- endif %}
{%- endif %}
{%- if e.is_non_exhaustive() -%}
non_exhaustive,
{%- endif %}
)]
enum r#{{ e.name() }} {
{%- for variant in e.variants() %}
Expand Down
100 changes: 82 additions & 18 deletions uniffi_macros/src/enum_.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
use proc_macro2::{Ident, Span, TokenStream};
use quote::quote;
use syn::{Data, DataEnum, DeriveInput, Expr, Field, Index, Lit, Variant};
use syn::{
parse::{Parse, ParseStream},
Data, DataEnum, DeriveInput, Expr, Field, Index, Lit, Variant,
};

use crate::util::{
create_metadata_items, derive_all_ffi_traits, extract_docstring, ident_to_string, mod_path,
tagged_impl_header, try_metadata_value_from_usize, try_read_field,
create_metadata_items, derive_all_ffi_traits, either_attribute_arg, extract_docstring,
ident_to_string, kw, mod_path, parse_comma_separated, tagged_impl_header,
try_metadata_value_from_usize, try_read_field, AttributeSliceExt, UniffiAttributeArgs,
};

pub fn expand_enum(input: DeriveInput, udl_mode: bool) -> syn::Result<TokenStream> {
pub fn expand_enum(
input: DeriveInput,
// Attributes from #[derive_error_for_udl()], if we are in udl mode
attr_from_udl_mode: Option<EnumAttr>,
udl_mode: bool,
) -> syn::Result<TokenStream> {
let enum_ = match input.data {
Data::Enum(e) => e,
_ => {
Expand All @@ -19,10 +28,14 @@ pub fn expand_enum(input: DeriveInput, udl_mode: bool) -> syn::Result<TokenStrea
};
let ident = &input.ident;
let docstring = extract_docstring(&input.attrs)?;
let ffi_converter_impl = enum_ffi_converter_impl(ident, &enum_, udl_mode);
let mut attr: EnumAttr = input.attrs.parse_uniffi_attr_args()?;
if let Some(attr_from_udl_mode) = attr_from_udl_mode {
attr = attr.merge(attr_from_udl_mode)?;
}
let ffi_converter_impl = enum_ffi_converter_impl(ident, &enum_, udl_mode, &attr);

let meta_static_var = (!udl_mode).then(|| {
enum_meta_static_var(ident, docstring, &enum_)
enum_meta_static_var(ident, docstring, &enum_, &attr)
.unwrap_or_else(syn::Error::into_compile_error)
});

Expand All @@ -36,11 +49,13 @@ pub(crate) fn enum_ffi_converter_impl(
ident: &Ident,
enum_: &DataEnum,
udl_mode: bool,
attr: &EnumAttr,
) -> TokenStream {
enum_or_error_ffi_converter_impl(
ident,
enum_,
udl_mode,
attr,
quote! { ::uniffi::metadata::codes::TYPE_ENUM },
)
}
Expand All @@ -49,11 +64,13 @@ pub(crate) fn rich_error_ffi_converter_impl(
ident: &Ident,
enum_: &DataEnum,
udl_mode: bool,
attr: &EnumAttr,
) -> TokenStream {
enum_or_error_ffi_converter_impl(
ident,
enum_,
udl_mode,
attr,
quote! { ::uniffi::metadata::codes::TYPE_ENUM },
)
}
Expand All @@ -62,6 +79,7 @@ fn enum_or_error_ffi_converter_impl(
ident: &Ident,
enum_: &DataEnum,
udl_mode: bool,
attr: &EnumAttr,
metadata_type_code: TokenStream,
) -> TokenStream {
let name = ident_to_string(ident);
Expand All @@ -71,19 +89,29 @@ fn enum_or_error_ffi_converter_impl(
Ok(p) => p,
Err(e) => return e.into_compile_error(),
};
let write_match_arms = enum_.variants.iter().enumerate().map(|(i, v)| {
let v_ident = &v.ident;
let fields = v.fields.iter().map(|f| &f.ident);
let idx = Index::from(i + 1);
let write_fields = v.fields.iter().map(write_field);
let mut write_match_arms: Vec<_> = enum_
.variants
.iter()
.enumerate()
.map(|(i, v)| {
let v_ident = &v.ident;
let fields = v.fields.iter().map(|f| &f.ident);
let idx = Index::from(i + 1);
let write_fields = v.fields.iter().map(write_field);

quote! {
Self::#v_ident { #(#fields),* } => {
::uniffi::deps::bytes::BufMut::put_i32(buf, #idx);
#(#write_fields)*
quote! {
Self::#v_ident { #(#fields),* } => {
::uniffi::deps::bytes::BufMut::put_i32(buf, #idx);
#(#write_fields)*
}
}
}
});
})
.collect();
if attr.non_exhaustive.is_some() {
write_match_arms.push(quote! {
_ => panic!("Unexpected variant in non-exhaustive enum"),
})
}
let write_impl = quote! {
match obj { #(#write_match_arms)* }
};
Expand Down Expand Up @@ -142,17 +170,22 @@ pub(crate) fn enum_meta_static_var(
ident: &Ident,
docstring: String,
enum_: &DataEnum,
attr: &EnumAttr,
) -> syn::Result<TokenStream> {
let name = ident_to_string(ident);
let module_path = mod_path()?;
let non_exhaustive = attr.non_exhaustive.is_some();

let mut metadata_expr = quote! {
::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::ENUM)
.concat_str(#module_path)
.concat_str(#name)
};
metadata_expr.extend(variant_metadata(enum_)?);
metadata_expr.extend(quote! { .concat_long_str(#docstring) });
metadata_expr.extend(quote! {
.concat_bool(#non_exhaustive)
.concat_long_str(#docstring)
});
Ok(create_metadata_items("enum", &name, metadata_expr, None))
}

Expand Down Expand Up @@ -234,3 +267,34 @@ pub fn variant_metadata(enum_: &DataEnum) -> syn::Result<Vec<TokenStream>> {
)
.collect()
}

#[derive(Default)]
pub struct EnumAttr {
pub non_exhaustive: Option<kw::non_exhaustive>,
}

// So ErrorAttr can be used with `parse_macro_input!`
impl Parse for EnumAttr {
fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
parse_comma_separated(input)
}
}

impl UniffiAttributeArgs for EnumAttr {
fn parse_one(input: ParseStream<'_>) -> syn::Result<Self> {
let lookahead = input.lookahead1();
if lookahead.peek(kw::non_exhaustive) {
Ok(Self {
non_exhaustive: input.parse()?,
})
} else {
Err(lookahead.error())
}
}

fn merge(self, other: Self) -> syn::Result<Self> {
Ok(Self {
non_exhaustive: either_attribute_arg(self.non_exhaustive, other.non_exhaustive)?,
})
}
}
Loading

0 comments on commit 2272b56

Please sign in to comment.