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.

Sign up for GitHub

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

Jump to bottom

feat(enum): u16/u32 discriminants #315

Closed
wants to merge 19 commits into from
Closed

feat(enum): u16/u32 discriminants #315

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
dca2e53
did most of u16 disciminator, failed on deserialize_variant
dzmitry-lahoda Oct 14, 2024
24a3be1
generally it works, but code is ugly
dzmitry-lahoda Oct 14, 2024
784755f
schema fixed and fmt
dzmitry-lahoda Oct 14, 2024
8914f06
basic macro redesign
dzmitry-lahoda Oct 26, 2024
557e240
very weird iterator
dzmitry-lahoda Oct 26, 2024
4b6cb92
in general works
dzmitry-lahoda Oct 26, 2024
df34e73
testing
dzmitry-lahoda Oct 27, 2024
0c2b17e
stash
dzmitry-lahoda Nov 3, 2024
58b8b14
flush
dzmitry-lahoda Nov 3, 2024
910bb08
Merge branch 'master' into dz/2
dzmitry-lahoda Nov 3, 2024
de5f32f
so we are backward compatible
dzmitry-lahoda Nov 3, 2024
3da00ce
semantic tests path
dzmitry-lahoda Nov 3, 2024
4acaec2
schema changes
dzmitry-lahoda Nov 3, 2024
affadf4
sign in schema
dzmitry-lahoda Nov 3, 2024
aef4b2e
fmt
dzmitry-lahoda Nov 3, 2024
5389767
clean up
dzmitry-lahoda Nov 3, 2024
bb457c7
fixed CI
dzmitry-lahoda Nov 3, 2024
8e1a8b2
oh more CI
dzmitry-lahoda Nov 3, 2024
6f67b47
Merge branch 'master' into dz/2
dzmitry-lahoda Nov 8, 2024
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
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ members = ["borsh", "borsh-derive", "fuzz/fuzz-run", "benchmarks"]
[workspace.package]
# shared version of all public crates in the workspace
version = "1.5.1"
rust-version = "1.67.0"
rust-version = "1.81.0"
dzmitry-lahoda marked this conversation as resolved.
Show resolved Hide resolved
19 changes: 15 additions & 4 deletions borsh-derive/src/internals/attributes/item/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::internals::attributes::{BORSH, CRATE, INIT, USE_DISCRIMINANT};
use quote::ToTokens;
use syn::{spanned::Spanned, Attribute, DeriveInput, Error, Expr, ItemEnum, Path};
use syn::{spanned::Spanned, Attribute, DeriveInput, Error, Expr, ItemEnum, Path, TypePath};

use super::{get_one_attribute, parsing};
use super::{get_one_attribute, parsing, REPR};

pub fn check_attributes(derive_input: &DeriveInput) -> Result<(), Error> {
let borsh = get_one_attribute(&derive_input.attrs)?;
Expand Down Expand Up @@ -34,10 +34,11 @@ pub fn check_attributes(derive_input: &DeriveInput) -> Result<(), Error> {
}

pub(crate) fn contains_use_discriminant(input: &ItemEnum) -> Result<bool, syn::Error> {
if input.variants.len() > 256 {
const MAX_VARIANTS: usize = u16::MAX as usize + 1;
if input.variants.len() > MAX_VARIANTS {
return Err(syn::Error::new(
input.span(),
"up to 256 enum variants are supported",
"up to {MAX_VARIANTS} enum variants are supported",
));
}

Expand Down Expand Up @@ -80,6 +81,16 @@ pub(crate) fn contains_use_discriminant(input: &ItemEnum) -> Result<bool, syn::E
Ok(use_discriminant.unwrap_or(false))
}

/// Gets type of reprc attribute if it exists
pub(crate) fn get_maybe_reprc_attribute(input: &ItemEnum) -> Option<TypePath> {
input
.attrs
.iter()
.find(|x| x.path() == REPR)?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, this behaviour should be controlled by borsh-specific item attribute, say #[borsh(enum_repr = "i16")],
not by repr, which is used to control memory layout in program, which is, strictly speaking, independent of how it's serialized for later transfer or saving to disk (which is what serialization is usually used for).
Say, an enum may not have #[repr(u16)] and have all discriminants withing u8 range, but still have #[borsh(enum_repr = "u16")] and use 2 bytes for tag serialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming we have borsh enum_repr, how should borsh handle this

fn main(){
    #[borsh(enum_repr = "u16")]
    #[repr(u32)]
    enum A {
        Q = 1u8,
    }
}

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may use u16 instead of "u16"? may just do borsh(rerp= instead of #[borsh(enum_repr` because it is on enum anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may it be like this?

    #[borsh(repr = u16)]
    enum A {
        Q = 1u8,
    }

and fail compile same way #repr fails because u8 was set (so borsh rerp interpreted types same way/algorithm) as native repr.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may it be like this?

looks good, repr should then check it annotates an enum item.

fails because u8 was set

well, one would've to change 1u8 to 1 to compile with borsh attribute.

Specifying Q = -10 would allow annotating with #[repr(i8)] and #[borsh(repr = i16)] at the same time

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may it be like this?

Still, maybe naming it enum_tag will be better, rust's repr has other subattributes, not related to enums at all

.parse_args()
.ok()
}

pub(crate) fn contains_initialize_with(attrs: &[Attribute]) -> Result<Option<Path>, Error> {
let mut res = None;
let attr = attrs.iter().find(|attr| attr.path() == BORSH);
Expand Down
1 change: 1 addition & 0 deletions borsh-derive/src/internals/attributes/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub const SERIALIZE_WITH: Symbol = Symbol("serialize_with", "serialize_with = ..
pub const DESERIALIZE_WITH: Symbol = Symbol("deserialize_with", "deserialize_with = ...");
/// crate - sub-borsh nested meta, item-level only, `BorshSerialize`, `BorshDeserialize`, `BorshSchema` contexts
pub const CRATE: Symbol = Symbol("crate", "crate = ...");
pub const REPR: Symbol = Symbol("repr", "repr(...)");

#[cfg(feature = "schema")]
pub mod schema_keys {
Expand Down
32 changes: 23 additions & 9 deletions borsh-derive/src/internals/deserialize/enums/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,25 @@ pub fn process(input: &ItemEnum, cratename: Path) -> syn::Result<TokenStream2> {
let mut where_clause = generics::default_where(where_clause);
let mut variant_arms = TokenStream2::new();
let use_discriminant = item::contains_use_discriminant(input)?;
let discriminants = Discriminants::new(&input.variants);
let maybe_reprc_attribute = use_discriminant
.then(|| item::get_maybe_reprc_attribute(input))
.flatten();
let discriminants: Discriminants = Discriminants::new(&input.variants, maybe_reprc_attribute);
let mut generics_output = deserialize::GenericsOutput::new(&generics);

let discriminant_type = discriminants.discriminant_type();
for (variant_idx, variant) in input.variants.iter().enumerate() {
let variant_body = process_variant(variant, &cratename, &mut generics_output)?;
let variant_ident = &variant.ident;

let discriminant_value = discriminants.get(variant_ident, use_discriminant, variant_idx)?;
variant_arms.extend(quote! {
if variant_tag == #discriminant_value { #name::#variant_ident #variant_body } else
if TryInto::<(#discriminant_type)>::try_into(variant_tag).map_err(|_|
return #cratename::io::Error::new(
#cratename::io::ErrorKind::InvalidData,
#cratename::__private::maybestd::format!("Unexpected variant tag: {:?}", variant_tag),
)
)?
== #discriminant_value { #name::#variant_ident #variant_body } else
});
}
let init = if let Some(method_ident) = item::contains_initialize_with(&input.attrs)? {
Expand All @@ -32,19 +41,23 @@ pub fn process(input: &ItemEnum, cratename: Path) -> syn::Result<TokenStream2> {
};
generics_output.extend(&mut where_clause, &cratename);

Ok(quote! {
let x = quote! {
impl #impl_generics #cratename::de::BorshDeserialize for #name #ty_generics #where_clause {
fn deserialize_reader<__R: #cratename::io::Read>(reader: &mut __R) -> ::core::result::Result<Self, #cratename::io::Error> {
let tag = <u8 as #cratename::de::BorshDeserialize>::deserialize_reader(reader)?;
<Self as #cratename::de::EnumExt>::deserialize_variant(reader, tag)
let tag = <#discriminant_type as #cratename::de::BorshDeserialize>::deserialize_reader(reader)?;
<Self as #cratename::de::EnumExt>::deserialize_variant::<_, #discriminant_type>(reader, tag)
}
}

impl #impl_generics #cratename::de::EnumExt for #name #ty_generics #where_clause {
fn deserialize_variant<__R: #cratename::io::Read>(
fn deserialize_variant<__R: #cratename::io::Read,
Tag: borsh::BorshDeserialize + ::core::fmt::Debug + Eq
+ ::core::convert::TryInto<u16> + ::core::convert::TryInto<u8> + Copy
>(
reader: &mut __R,
variant_tag: u8,
variant_tag: Tag,
) -> ::core::result::Result<Self, #cratename::io::Error> {
use ::core::convert::TryInto;
let mut return_value =
#variant_arms {
return Err(#cratename::io::Error::new(
Expand All @@ -56,7 +69,8 @@ pub fn process(input: &ItemEnum, cratename: Path) -> syn::Result<TokenStream2> {
Ok(return_value)
}
}
})
};
Ok(x)
}

fn process_variant(
Expand Down
102 changes: 102 additions & 0 deletions borsh-derive/src/internals/deserialize/enums/snapshots/borsh_discriminant_false.snap.new
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
---
source: borsh-derive/src/internals/deserialize/enums/mod.rs
assertion_line: 309
expression: pretty_print_syn_str(&actual).unwrap()
---
impl borsh::de::BorshDeserialize for X {
fn deserialize_reader<__R: borsh::io::Read>(
reader: &mut __R,
) -> ::core::result::Result<Self, borsh::io::Error> {
let tag = <u8 as borsh::de::BorshDeserialize>::deserialize_reader(reader)?;
<Self as borsh::de::EnumExt>::deserialize_variant::<_, u8>(reader, tag)
}
}
impl borsh::de::EnumExt for X {
fn deserialize_variant<
__R: borsh::io::Read,
Tag: borsh::BorshDeserialize + ::core::fmt::Debug + Eq
+ ::core::convert::TryInto<u16> + ::core::convert::TryInto<u8> + Copy,
>(
reader: &mut __R,
variant_tag: Tag,
) -> ::core::result::Result<Self, borsh::io::Error> {
use ::core::convert::TryInto;
let mut return_value = if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 0u8
{
X::A
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 1u8
{
X::B
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 2u8
{
X::C
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 3u8
{
X::D
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 4u8
{
X::E
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 5u8
{
X::F
} else {
return Err(
borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
),
)
};
Ok(return_value)
}
}
102 changes: 102 additions & 0 deletions borsh-derive/src/internals/deserialize/enums/snapshots/borsh_discriminant_true.snap.new
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
---
source: borsh-derive/src/internals/deserialize/enums/mod.rs
assertion_line: 327
expression: pretty_print_syn_str(&actual).unwrap()
---
impl borsh::de::BorshDeserialize for X {
fn deserialize_reader<__R: borsh::io::Read>(
reader: &mut __R,
) -> ::core::result::Result<Self, borsh::io::Error> {
let tag = <u8 as borsh::de::BorshDeserialize>::deserialize_reader(reader)?;
<Self as borsh::de::EnumExt>::deserialize_variant::<_, u8>(reader, tag)
}
}
impl borsh::de::EnumExt for X {
fn deserialize_variant<
__R: borsh::io::Read,
Tag: borsh::BorshDeserialize + ::core::fmt::Debug + Eq
+ ::core::convert::TryInto<u16> + ::core::convert::TryInto<u8> + Copy,
>(
reader: &mut __R,
variant_tag: Tag,
) -> ::core::result::Result<Self, borsh::io::Error> {
use ::core::convert::TryInto;
let mut return_value = if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 0
{
X::A
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 20
{
X::B
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 20 + 1
{
X::C
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 20 + 1 + 1
{
X::D
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 10
{
X::E
} else if TryInto::<(u8)>::try_into(variant_tag)
.map_err(|_| {
return borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
);
})? == 10 + 1
{
X::F
} else {
return Err(
borsh::io::Error::new(
borsh::io::ErrorKind::InvalidData,
borsh::__private::maybestd::format!(
"Unexpected variant tag: {:?}", variant_tag
),
),
)
};
Ok(return_value)
}
}
Loading
Loading