diff --git a/CHANGELOG.md b/CHANGELOG.md index 88a079e..ab91acd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,11 @@ # 0.5 + * All enums now default to `#[protocol(discriminant = "string")]` + * This discriminator type allows the most flexibility in future protocol changes. + * The old behaviour can still be achieved via `#[protocol(discriminant = "integer")]` * Add `protocol::Settings` to Parcel API for byte order configuration * Every `Parcel` implementation must be adjusted to include the new argument + * The `#[repr(integer type)]` enum attribute is now recognized and respected. # 0.4 and below diff --git a/README.md b/README.md index b372cf0..f143e32 100644 --- a/README.md +++ b/README.md @@ -115,18 +115,19 @@ fn main() { Enum values can be transmitted either by their 1-based variant index, or by transmitting the string name of each variant. -**NOTE:** The default behaviour is to use *the 1-based variant index* (`integer`). +**NOTE:** The default behaviour is to use *the variant name as a string* (`string`). -This behaviour can be changed by the `#[protocol(discriminant = "string")]` attribute. +This behaviour can be changed by the `#[protocol(discriminant = "")]` attribute. Supported discriminant types: -* `integer` (default) - * This transmits the 1-based variant number as the over-the-wire discriminant - * Enum variants cannot be reordered in the source without breaking the protocol -* `string` +* `string` (default) * This transmits the enum variant name as the over-the-wire discriminant * This uses more bytes per message, but it very flexible +* `integer + * This transmits the 1-based variant number as the over-the-wire discriminant + * If enum variants have explicit discriminators, the + * Enum variants cannot be reordered in the source without breaking the protocol **N.B.** `string` should become the default discriminant type for `#[derive(Protocol)]`. This would be a breaking change. Perhaps a major release? diff --git a/protocol-derive-tests/src/enums.rs b/protocol-derive-tests/src/enums.rs index 6b6f52c..960181a 100644 --- a/protocol-derive-tests/src/enums.rs +++ b/protocol-derive-tests/src/enums.rs @@ -113,6 +113,19 @@ mod integer_discriminants { Second = 2, } + #[derive(Protocol, Debug, PartialEq, Eq)] + #[protocol(discriminant = "integer")] + #[repr(i8)] + enum WithoutExplicitDiscriminators { + Only, + } + + #[test] + fn discriminator_zero_is_reserved() { + assert_eq!(vec![1], + WithoutExplicitDiscriminators::Only.raw_bytes(&protocol::Settings::default()).unwrap()); + } + #[test] fn named_fields_are_correctly_written() { assert_eq!(vec![0, 0, 0, 1, 1], BoatKind::Speedboat { diff --git a/protocol-derive/src/format.rs b/protocol-derive/src/format.rs index b464f75..94c2e5c 100644 --- a/protocol-derive/src/format.rs +++ b/protocol-derive/src/format.rs @@ -5,6 +5,8 @@ use syn; pub const DEFAULT_INT_DISCRIMINATOR_TYPE: &'static str = "u32"; +pub const DEFAULT_ENUM_DISCRIMINATOR_FORMAT: Enum = Enum::StringDiscriminator; + /// Represents a format. pub trait Format : Clone { /// From a string. @@ -24,14 +26,37 @@ impl Enum { /// Gets the discriminator of an enum variant. pub fn discriminator(&self, e: &syn::DataEnum, variant: &syn::Variant) -> ::proc_macro2::TokenStream { + let allow_explicit_discriminators = match *self { + Enum::IntegerDiscriminator => true, + _ => false, + }; + + if !allow_explicit_discriminators { + for variant in e.variants.iter() { + if let Some(_) = variant.discriminant { + panic!("only enums with integer discriminants may use explicit discriminants: '{}", + quote!(#variant).to_string()); + } + } + } + match *self { Enum::IntegerDiscriminator => { let variant_index = e.variants.iter().position(|v| v.ident == variant.ident).expect("variant not a part of enum"); + let prior_variants: Vec<_> = e.variants.iter().collect(); + let previous_discriminator = prior_variants.into_iter().rev().filter_map(|variant| { + match variant.discriminant { + Some((_, syn::Expr::Lit(syn::ExprLit { lit: syn::Lit::Int(ref n), .. }))) => Some(n.value()), + _ => None, + } + }).next() + .unwrap_or_else(|| variant_index as u64 + 1); // incase no explicit discriminators + let default_discriminator = previous_discriminator + 1; let discriminator = match variant.discriminant { Some((_, syn::Expr::Lit(syn::ExprLit { lit: syn::Lit::Int(ref n), .. }))) => n.clone(), Some(_) => panic!("unknown discriminator"), - // Reserve discriminator 0. + // If no explicit discriminant exists None => syn::LitInt::new(variant_index as u64 + 1, syn::IntSuffix::None, proc_macro2::Span::call_site()), }; diff --git a/protocol-derive/src/lib.rs b/protocol-derive/src/lib.rs index 470c063..af62aca 100644 --- a/protocol-derive/src/lib.rs +++ b/protocol-derive/src/lib.rs @@ -178,7 +178,8 @@ fn impl_parcel_for_enum(ast: &syn::DeriveInput, let enum_name = &ast.ident; let anon_const_name = syn::Ident::new(&format!("__IMPL_PARCEL_FOR_{}", ast.ident), proc_macro2::Span::call_site()); - let format = attr::discriminant_format::(&ast.attrs).unwrap_or(format::Enum::IntegerDiscriminator); + let format = attr::discriminant_format::(&ast.attrs) + .unwrap_or(format::DEFAULT_ENUM_DISCRIMINATOR_FORMAT); let discriminator_ty = match format { format::Enum::IntegerDiscriminator => {