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

Follow-up for canonical se/de after an additional round of review #588

Merged
merged 4 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
10 changes: 8 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,19 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Added

- [#586](https://github.com/FuelLabs/fuel-vm/pull/586): Added `default_asset` method to the `ContractIdExt` trait implementation, to mirror the `default` method on AssetId in the Sway std lib.

### Changed
#### Breaking

- [#578](https://github.com/FuelLabs/fuel-vm/pull/578): Support `no_std` environments for `fuel-crypto`, falling back to a pure-Rust crypto implementation.
- [#582](https://github.com/FuelLabs/fuel-vm/pull/582): Make `fuel-vm` and `fuel-tx` crates compatible with `no_std` + `alloc`. This includes reworking all error handling that used `std::io::Error`, replacing some `std::collection::{HashMap, HashSet}` with `hashbrown::{HashMap, HashSet}` and many changes to feature-gating of APIs.
### Added
- [#588](https://github.com/FuelLabs/fuel-vm/pull/588): Removed `SerialziedSize` and `SerialziedFixedSize` traits. Removed support for `SIZE_NO_DYNAMIC` and `SIZE_STATIC`. Removed enum attributes from derive macro for `Serialize` and `Deserialize` traits.
xgreenx marked this conversation as resolved.
Show resolved Hide resolved

- [#586](https://github.com/FuelLabs/fuel-vm/pull/586): Added `default_asset` method to the `ContractIdExt` trait implementation, to mirror the `default` method on AssetId in the Sway std lib.
#### Removed
- [#588](https://github.com/FuelLabs/fuel-vm/pull/588): .
xgreenx marked this conversation as resolved.
Show resolved Hide resolved

## [Version 0.37.0]

Expand Down
68 changes: 0 additions & 68 deletions fuel-derive/src/attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ fn parse_attrs(s: &synstructure::Structure) -> HashMap<String, TokenStream> {
/// Pop-level `canonical` attributes for a struct
pub struct StructAttrs {
/// The struct is prefixed with the given word.
/// Useful with`#[canonical(inner_discriminant)]`.
/// The type must implement `Into<u64>`, which is used to serialize it.
pub prefix: Option<TokenStream>,
}
Expand All @@ -76,73 +75,6 @@ impl StructAttrs {
}
}

/// Pop-level `canonical` attributes for an enum
#[allow(non_snake_case)]
pub struct EnumAttrs {
/// This is a wrapper enum where every variant can be recognized from it's first
/// word. This means that the enum itself doesn't have to serialize the
/// discriminant, but the field itself does so. This can be done using
/// `#[canonical(prefix = ...)]` attribute. `TryFromPrimitive` traits are used to
/// convert the raw bytes into the given type.
pub inner_discriminant: Option<TokenStream>,
/// Replaces calculation of the serialized static size with a custom function.
pub serialized_size_static_with: Option<TokenStream>,
/// Replaces calculation of the serialized dynamic size with a custom function.
pub serialized_size_dynamic_with: Option<TokenStream>,
/// Replaces serialization with a custom function.
pub serialize_with: Option<TokenStream>,
/// Replaces deserialization with a custom function.
pub deserialize_with: Option<TokenStream>,
/// Determines whether the enum has a dynamic size when `serialize_with` is used.
pub SIZE_NO_DYNAMIC: Option<TokenStream>,
}

impl EnumAttrs {
#[allow(non_snake_case)]
pub fn parse(s: &synstructure::Structure) -> Self {
let mut attrs = parse_attrs(s);

let inner_discriminant = attrs.remove("inner_discriminant");
let serialized_size_static_with = attrs.remove("serialized_size_static_with");
let serialized_size_dynamic_with = attrs.remove("serialized_size_dynamic_with");
let serialize_with = attrs.remove("serialize_with");
let deserialize_with = attrs.remove("deserialize_with");
let SIZE_NO_DYNAMIC = attrs.remove("SIZE_NO_DYNAMIC");

if !attrs.is_empty() {
panic!("unknown canonical attributes: {:?}", attrs.keys())
}

Self {
inner_discriminant,
serialized_size_static_with,
serialized_size_dynamic_with,
serialize_with,
deserialize_with,
SIZE_NO_DYNAMIC,
}
}
}

/// Parse `#[repr(int)]` attribute for an enum.
pub fn parse_enum_repr(attrs: &[Attribute]) -> Option<String> {
for attr in attrs {
if attr.style != AttrStyle::Outer {
continue
}
if let Meta::List(ml) = &attr.meta {
if ml.path.segments.len() == 1 && ml.path.segments[0].ident == "repr" {
if let Some(TokenTree::Ident(ident)) =
ml.tokens.clone().into_iter().next()
{
return Some(ident.to_string())
}
}
}
}
None
}

/// Parse `#[canonical(skip)]` attribute for a binding field.
pub fn should_skip_field_binding(binding: &BindingInfo<'_>) -> bool {
should_skip_field(&binding.ast().attrs)
Expand Down
96 changes: 24 additions & 72 deletions fuel-derive/src/deserialize.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
use proc_macro2::TokenStream as TokenStream2;
use quote::quote;

use crate::{
attribute::{
should_skip_field,
should_skip_field_binding,
EnumAttrs,
StructAttrs,
},
evaluate::evaluate_simple_expr,
use crate::attribute::{
should_skip_field,
should_skip_field_binding,
StructAttrs,
};

fn deserialize_struct(s: &mut synstructure::Structure) -> TokenStream2 {
Expand Down Expand Up @@ -42,8 +38,8 @@ fn deserialize_struct(s: &mut synstructure::Structure) -> TokenStream2 {

let remove_prefix = if let Some(expected_prefix) = StructAttrs::parse(s).prefix {
quote! {{
let prefix = <u64 as ::fuel_types::canonical::Deserialize>::decode_static(buffer)?;
if prefix.try_into() != Ok(#expected_prefix) {
let prefix = <_ as ::fuel_types::canonical::Deserialize>::decode_static(buffer);
if prefix != Ok(#expected_prefix) {
return ::core::result::Result::Err(::fuel_types::canonical::Error::InvalidPrefix)
}
}}
Expand All @@ -69,12 +65,11 @@ fn deserialize_struct(s: &mut synstructure::Structure) -> TokenStream2 {
}

fn deserialize_enum(s: &synstructure::Structure) -> TokenStream2 {
let attrs = EnumAttrs::parse(s);
let _name = &s.ast().ident;

assert!(!s.variants().is_empty(), "got invalid empty enum");

let mut next_discriminant = 0u64;
let mut next_discriminant = quote! { { 0u64 } };
let decode_static: TokenStream2 = s
.variants()
.iter()
Expand All @@ -92,36 +87,30 @@ fn deserialize_enum(s: &synstructure::Structure) -> TokenStream2 {
}
});

if variant.ast().discriminant.is_some() {
Voxelot marked this conversation as resolved.
Show resolved Hide resolved
let variant_ident = variant.ast().ident;
next_discriminant = quote! { { Self::#variant_ident as u64 } };
}

let discr = if let Some(discr_type) = attrs.inner_discriminant.as_ref() {
let vname = variant.ast().ident;
quote! { #discr_type::#vname }
} else {
if let Some((_, d)) = variant.ast().discriminant {
next_discriminant = evaluate_simple_expr(d).expect("Unable to evaluate discriminant expression");
};
let v = next_discriminant;
next_discriminant = next_discriminant.checked_add(1).expect("Discriminant overflow");
quote! { #v }
};

quote! {
#discr => {
let result = quote! {
x if x == #next_discriminant => {
::core::result::Result::Ok(#decode_main)
}
}
};

next_discriminant = quote! { ( (#next_discriminant) + 1u64 ) };

result
}).collect();

let decode_dynamic = s.variants().iter().map(|variant| {
let decode_dynamic = variant.each(|binding| {
if should_skip_field_binding(binding) {
quote! {
*#binding = ::core::default::Default::default();
}
} else {
if !should_skip_field_binding(binding) {
quote! {{
::fuel_types::canonical::Deserialize::decode_dynamic(#binding, buffer)?;
}}
} else {
quote! {}
}
});

Expand All @@ -130,53 +119,16 @@ fn deserialize_enum(s: &synstructure::Structure) -> TokenStream2 {
}
});

// Handle #[canonical(inner_discriminant = Type)]
let decode_discriminant = if attrs.inner_discriminant.is_some() {
let discriminant = {
quote! {
let buf = buffer.clone();
let raw_discr = <::core::primitive::u64 as ::fuel_types::canonical::Deserialize>::decode(buffer)?;
*buffer = buf; // Restore buffer position
<::core::primitive::u64 as ::fuel_types::canonical::Deserialize>::decode(buffer)?
}
} else {
quote! {
let raw_discr = <::core::primitive::u64 as ::fuel_types::canonical::Deserialize>::decode(buffer)?;
}
};

// Handle #[canonical(inner_discriminant = Type)]
let mapped_discr = if let Some(discr_type) = attrs.inner_discriminant {
quote! { {
use ::num_enum::{TryFromPrimitive, IntoPrimitive};
let Ok(discr) = #discr_type::try_from_primitive(raw_discr) else {
return ::core::result::Result::Err(::fuel_types::canonical::Error::UnknownDiscriminant)
};
discr
} }
} else {
quote! { raw_discr }
};

// Handle #[canonical(deserialize_with = function)]
if let Some(helper) = attrs.deserialize_with {
return s.gen_impl(quote! {
gen impl ::fuel_types::canonical::Deserialize for @Self {
fn decode_static<I: ::fuel_types::canonical::Input + ?Sized>(buffer: &mut I) -> ::core::result::Result<Self, ::fuel_types::canonical::Error> {
let raw_discr = <::core::primitive::u64 as ::fuel_types::canonical::Deserialize>::decode(buffer)?;
#helper(#mapped_discr, buffer)
}

fn decode_dynamic<I: ::fuel_types::canonical::Input + ?Sized>(&mut self, buffer: &mut I) -> ::core::result::Result<(), ::fuel_types::canonical::Error> {
::core::result::Result::Ok(())
}
}
})
}

s.gen_impl(quote! {
gen impl ::fuel_types::canonical::Deserialize for @Self {
fn decode_static<I: ::fuel_types::canonical::Input + ?Sized>(buffer: &mut I) -> ::core::result::Result<Self, ::fuel_types::canonical::Error> {
#decode_discriminant
match #mapped_discr {
match #discriminant {
#decode_static
_ => ::core::result::Result::Err(::fuel_types::canonical::Error::UnknownDiscriminant),
}
Expand Down
14 changes: 0 additions & 14 deletions fuel-derive/src/evaluate.rs

This file was deleted.

4 changes: 0 additions & 4 deletions fuel-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,8 @@
#![deny(unused_must_use, missing_docs)]

extern crate proc_macro;

mod utils;

mod attribute;
mod deserialize;
mod evaluate;
mod serialize;

use self::{
Expand Down
Loading
Loading