Skip to content

Commit

Permalink
Make variant index explicit, remove discriminant (#112)
Browse files Browse the repository at this point in the history 
* Make variant index explicit

* Remove discriminant, replace with explicit index
  • Loading branch information
@ascjones
ascjones authored Jul 15, 2021
1 parent 53c7cfc commit 1608467
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 105 deletions.
13 changes: 7 additions & 6 deletions derive/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,12 @@ fn generate_c_like_enum_def(variants: &VariantList, scale_info: &Ident) -> Token
.filter(|(_, v)| !utils::should_skip(&v.attrs))
.map(|(i, v)| {
let name = &v.ident;
let discriminant = utils::variant_index(v, i);
let index = utils::variant_index(v, i);
let docs = generate_docs(&v.attrs);
quote! {
.variant(::core::stringify!(#name), |v|
v
.discriminant(#discriminant as ::core::primitive::u64)
.index(#index as ::core::primitive::u8)
#docs
)
}
Expand Down Expand Up @@ -259,12 +259,13 @@ fn generate_variant_type(data_enum: &DataEnum, scale_info: &Ident) -> TokenStrea

let variants = variants
.into_iter()
.filter(|v| !utils::should_skip(&v.attrs))
.map(|v| {
.enumerate()
.filter(|(_, v)| !utils::should_skip(&v.attrs))
.map(|(i, v)| {
let ident = &v.ident;
let v_name = quote! {::core::stringify!(#ident) };
let docs = generate_docs(&v.attrs);
let index = utils::maybe_index(v).map(|i| quote!(.index(#i)));
let index = utils::variant_index(v, i);

let fields = match v.fields {
Fields::Named(ref fs) => {
Expand All @@ -291,9 +292,9 @@ fn generate_variant_type(data_enum: &DataEnum, scale_info: &Ident) -> TokenStrea
quote! {
.variant(#v_name, |v|
v
.index(#index as ::core::primitive::u8)
.fields(#fields)
#docs
#index
)
}
});
Expand Down
67 changes: 46 additions & 21 deletions src/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,15 @@
//! .type_params(type_params!(T))
//! .variant(
//! Variants::new()
//! .variant("A", |v| v.fields(Fields::unnamed().field(|f| f.ty::<T>().type_name("T"))))
//! .variant("B", |v| v.fields(Fields::named().field(|f| f.ty::<u32>().name("f").type_name("u32"))))
//! .variant_unit("A")
//! .variant("A", |v| v
//! .index(0)
//! .fields(Fields::unnamed().field(|f| f.ty::<T>().type_name("T")))
//! )
//! .variant("B", |v| v
//! .index(1)
//! .fields(Fields::named().field(|f| f.ty::<u32>().name("f").type_name("u32")))
//! )
//! .variant_unit("A", 2)
//! )
//! }
//! }
Expand All @@ -108,9 +114,9 @@
//! .path(Path::new("Foo", module_path!()))
//! .variant(
//! Variants::new()
//! .variant("A", |v| v.discriminant(1))
//! .variant("B", |v| v.discriminant(2))
//! .variant("C", |v| v.discriminant(33))
//! .variant("A", |v| v.index(1))
//! .variant("B", |v| v.index(2))
//! .variant("C", |v| v.index(33))
//! )
//! }
//! }
Expand All @@ -135,7 +141,7 @@ use crate::{
Variant,
};

/// State types for type builders which require a Path
/// State types for type builders which require a Path.
pub mod state {
/// State where the builder has not assigned a Path to the type
pub enum PathNotAssigned {}
Expand Down Expand Up @@ -433,16 +439,16 @@ impl Variants {
/// Add a variant
pub fn variant<F>(mut self, name: &'static str, builder: F) -> Self
where
F: Fn(VariantBuilder) -> VariantBuilder,
F: Fn(VariantBuilder) -> VariantBuilder<variant_state::IndexAssigned>,
{
let builder = builder(VariantBuilder::new(name));
self.variants.push(builder.finalize());
self
}

/// Add a unit variant (without fields).
pub fn variant_unit(mut self, name: &'static str) -> Self {
let builder = VariantBuilder::new(name);
pub fn variant_unit(mut self, name: &'static str, index: u8) -> Self {
let builder = VariantBuilder::new(name).index(index);
self.variants.push(builder.finalize());
self
}
Expand All @@ -453,16 +459,25 @@ impl Variants {
}
}

/// State types for the `VariantBuilder` which requires an index.
pub mod variant_state {
/// State where the builder has not assigned an index to a variant.
pub enum IndexNotAssigned {}
/// State where the builder has assigned an index to a variant.
pub enum IndexAssigned {}
}

/// Build a [`Variant`].
pub struct VariantBuilder {
pub struct VariantBuilder<S = variant_state::IndexNotAssigned> {
name: &'static str,
fields: Vec<Field<MetaForm>>,
index: Option<u8>,
fields: Vec<Field<MetaForm>>,
discriminant: Option<u64>,
docs: Vec<&'static str>,
marker: PhantomData<S>,
}

impl VariantBuilder {
impl VariantBuilder<variant_state::IndexNotAssigned> {
/// Create a new [`VariantBuilder`].
pub fn new(name: &'static str) -> Self {
Self {
Expand All @@ -471,21 +486,30 @@ impl VariantBuilder {
discriminant: None,
index: None,
docs: Vec::new(),
marker: Default::default(),
}
}

/// Set the variant's codec index.
pub fn index(self, index: u8) -> VariantBuilder<variant_state::IndexAssigned> {
VariantBuilder {
name: self.name,
index: Some(index),
fields: self.fields,
discriminant: self.discriminant,
docs: self.docs,
marker: Default::default(),
}
}
}

impl<S> VariantBuilder<S> {
/// Set the variant's discriminant.
pub fn discriminant(mut self, discriminant: u64) -> Self {
self.discriminant = Some(discriminant);
self
}

/// Set the variant's codec index.
pub fn index(mut self, index: u8) -> Self {
self.index = Some(index);
self
}

/// Initialize the variant's fields.
pub fn fields<F>(mut self, fields_builder: FieldsBuilder<F>) -> Self {
self.fields = fields_builder.finalize();
Expand All @@ -497,14 +521,15 @@ impl VariantBuilder {
self.docs = docs.to_vec();
self
}
}

impl VariantBuilder<variant_state::IndexAssigned> {
/// Complete building and create final [`Variant`] instance.
pub fn finalize(self) -> Variant<MetaForm> {
Variant::new(
self.name,
self.fields,
self.index,
self.discriminant,
self.index.expect("Index should be assigned by the builder"),
self.docs,
)
}
Expand Down
16 changes: 11 additions & 5 deletions src/impls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,13 @@ where
Type::builder()
.path(Path::prelude("Option"))
.type_params(type_params![T])
.variant(Variants::new().variant("None", |v| v).variant("Some", |v| {
v.fields(Fields::unnamed().field(|f| f.ty::<T>()))
}))
.variant(
Variants::new()
.variant("None", |v| v.index(0))
.variant("Some", |v| {
v.index(1).fields(Fields::unnamed().field(|f| f.ty::<T>()))
}),
)
}
}

Expand All @@ -206,9 +210,11 @@ where
.type_params(type_params!(T, E))
.variant(
Variants::new()
.variant("Ok", |v| v.fields(Fields::unnamed().field(|f| f.ty::<T>())))
.variant("Ok", |v| {
v.index(0).fields(Fields::unnamed().field(|f| f.ty::<T>()))
})
.variant("Err", |v| {
v.fields(Fields::unnamed().field(|f| f.ty::<E>()))
v.index(1).fields(Fields::unnamed().field(|f| f.ty::<E>()))
}),
)
}
Expand Down
20 changes: 13 additions & 7 deletions src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,13 @@ fn prelude_items() {
Type::builder()
.path(Path::prelude("Option"))
.type_params(named_type_params![(T, u128)])
.variant(Variants::new().variant("None", |v| v).variant("Some", |v| {
v.fields(Fields::unnamed().field(|f| f.ty::<u128>()))
}))
.variant(Variants::new().variant("None", |v| v.index(0)).variant(
"Some",
|v| {
v.index(1)
.fields(Fields::unnamed().field(|f| f.ty::<u128>()))
}
))
);
assert_type!(
Result<bool, String>,
Expand All @@ -75,10 +79,12 @@ fn prelude_items() {
Variants::new()
.variant(
"Ok", |v| v
.index(0)
.fields(Fields::unnamed().field(|f| f.ty::<bool>()))
)
.variant(
"Err", |v| v
.index(1)
.fields(Fields::unnamed().field(|f| f.ty::<String>()))
)
)
Expand Down Expand Up @@ -315,13 +321,13 @@ fn basic_enum_with_index() {
)
})
.variant("C", |v| {
v.fields(
v.index(2).fields(
Fields::unnamed()
.field(|f| f.ty::<u16>().type_name("u16"))
.field(|f| f.ty::<u32>().type_name("u32")),
)
})
.variant_unit("D"),
.variant_unit("D", 3),
)
}
}
Expand All @@ -341,13 +347,13 @@ fn basic_enum_with_index() {
)
})
.variant("C", |v| {
v.fields(
v.index(2).fields(
Fields::unnamed()
.field(|f| f.ty::<u16>().type_name("u16"))
.field(|f| f.ty::<u32>().type_name("u32")),
)
})
.variant_unit("D"),
.variant_unit("D", 3),
);

assert_type!(IndexedRustEnum, ty);
Expand Down
36 changes: 8 additions & 28 deletions src/ty/variant.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,24 +157,12 @@ pub struct Variant<T: Form = MetaForm> {
serde(skip_serializing_if = "Vec::is_empty", default)
)]
fields: Vec<Field<T>>,
/// Index of the variant, used in `parity-scale-codec`
#[cfg_attr(
feature = "serde",
serde(skip_serializing_if = "Option::is_none", default)
)]
index: Option<u8>,
/// The discriminant of the variant.
/// Index of the variant, used in `parity-scale-codec`.
///
/// # Note
///
/// Even though setting the discriminant is optional
/// every C-like enum variant has a discriminant specified
/// upon compile-time.
#[cfg_attr(
feature = "serde",
serde(skip_serializing_if = "Option::is_none", default)
)]
discriminant: Option<u64>,
/// The value of this will be, in order of precedence:
/// 1. The explicit index defined by a `#[codec(index = N)]` attribute.
/// 2. The implicit index from the position of the variant in the `enum` definition.
index: u8,
/// Documentation
#[cfg_attr(
feature = "serde",
Expand All @@ -191,7 +179,6 @@ impl IntoPortable for Variant {
name: self.name.into_portable(registry),
fields: registry.map_into_portable(self.fields),
index: self.index,
discriminant: self.discriminant,
docs: registry.map_into_portable(self.docs),
}
}
Expand All @@ -202,15 +189,13 @@ impl Variant {
pub(crate) fn new(
name: &'static str,
fields: Vec<Field<MetaForm>>,
index: Option<u8>,
discriminant: Option<u64>,
index: u8,
docs: Vec<&'static str>,
) -> Self {
Self {
name,
fields,
index,
discriminant,
docs,
}
}
Expand All @@ -230,16 +215,11 @@ where
&self.fields
}

/// Returns the index of the variant, if specified.
pub fn index(&self) -> Option<u8> {
/// Returns the index of the variant.
pub fn index(&self) -> u8 {
self.index
}

/// Returns the discriminant of the variant.
pub fn discriminant(&self) -> Option<u64> {
self.discriminant
}

/// Returns the documentation of the variant.
pub fn docs(&self) -> &[T::String] {
&self.docs
Expand Down
Loading

0 comments on commit 1608467

Please sign in to comment.