paritytech / scale-info

Info about SCALE encodable Rust types
Apache License 2.0
72 stars 30 forks source link

How to represent `BitFlags`? #119

Open ascjones opened 3 years ago

ascjones commented 3 years ago

In substrate we have the following type:

#[repr(u64)]
#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, BitFlags, RuntimeDebug, TypeInfo)]
pub enum IdentityField {
    Display = 0b0000000000000000000000000000000000000000000000000000000000000001,
    Legal = 0b0000000000000000000000000000000000000000000000000000000000000010,
    Web = 0b0000000000000000000000000000000000000000000000000000000000000100,
    Riot = 0b0000000000000000000000000000000000000000000000000000000000001000,
    Email = 0b0000000000000000000000000000000000000000000000000000000000010000,
    PgpFingerprint = 0b0000000000000000000000000000000000000000000000000000000000100000,
    Image = 0b0000000000000000000000000000000000000000000000000000000001000000,
    Twitter = 0b0000000000000000000000000000000000000000000000000000000010000000,
}

It is wrapped in a BitFlags in the following wrapper type to allow custom encoding/decoding.

/// Wrapper type for `BitFlags<IdentityField>` that implements `Codec`.
#[derive(Clone, Copy, PartialEq, Default, RuntimeDebug)]
pub struct IdentityFields(pub(crate) BitFlags<IdentityField>);

Question is how to represent this with scale_info: it is encoded as a raw u64 but we need to also have access to the IdentityField enum definition which maps the discriminants reduced to u8 indices.

A temporary workaround is to provide the following TypeInfo implementation

impl TypeInfo for IdentityFields {
    type Identity = Self;

    fn type_info() -> Type {
        Type::builder()
            .path(Path::new("BitFlags", module_path!()))
            .type_params(vec![ TypeParameter::new("T", Some(meta_type::<IdentityField>())) ])
            .composite(
            Fields::unnamed()
                .field(|f| f.ty::<u64>().type_name("IdentityField")),
        )
    }
}

See how it represents BitFlags<IdentityField> including the IdentityField as a type parameter, and the field itself as type u64.

However this requires some string matching for downstream type generators, so a preferable solution might be to add fist class support for a BitFlags like TypeDef variant.

On the other we might not want to be adding a new TypeDef variant for each corner case, be curious to see what others think.

/cc @jacogr

ascjones commented 3 years ago

Another possibility is to capture the #[repr(u64)] and encode that into an optional repr field in TypeDefVariant

gui1117 commented 3 years ago

sidenote: the Encode/Decode implementation on IdentityField is wrong and should be removed https://github.com/paritytech/substrate/pull/9445

ascjones commented 3 years ago

Then should I reconsider the removal of discriminant: u64 in https://github.com/paritytech/scale-info/pull/112?

gui1117 commented 3 years ago

if the only use of discriminant is bitflag maybe it is better to have dedicated syntax.

But otherwise it is true that the discriminant gives useful information in this specific case.