Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make variant index explicit, remove discriminant #112

Merged
merged 2 commits into from
Jul 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions derive/src/lib.rs
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
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 @@ -429,16 +435,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 @@ -449,16 +455,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 @@ -467,21 +482,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 @@ -493,14 +517,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
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,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 @@ -207,9 +211,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
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 @@ -301,13 +307,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 @@ -327,13 +333,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
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