Skip to content

Commit

Permalink
remove should_continue and other things
Browse files Browse the repository at this point in the history
  • Loading branch information
tadeohepperle committed Dec 7, 2023
1 parent 119bf73 commit cfec37d
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 75 deletions.
7 changes: 3 additions & 4 deletions description/src/description.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use anyhow::anyhow;
use scale_info::{
form::PortableForm, Field, PortableRegistry, Type, TypeDef, TypeDefArray, TypeDefBitSequence,
TypeDefCompact, TypeDefPrimitive, TypeDefSequence, TypeDefTuple, TypeDefVariant, Variant,
Expand Down Expand Up @@ -31,15 +30,15 @@ pub fn type_description(
}

fn return_type_name_on_cache_hit(
type_id: u32,
_type_id: u32,
ty: &Type<PortableForm>,
cached: &String,
transformer: &Transformer<String>,
_transformer: &Transformer<String>,
) -> Option<anyhow::Result<String>> {
if let Some(type_name) = ty.path.ident() {
return Some(Ok(type_name));
}
Some(Ok(cached.clone()))
Some(Ok(cached.to_owned()))
}
let transformer = Transformer::new(
ty_description,
Expand Down
6 changes: 3 additions & 3 deletions description/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ mod tests {
corners: u8,
radius: u64,
},
MultiShape {
Multi {
shapes: Vec<Shape<u64>>,
t: T,
operation: Operation,
Expand All @@ -122,7 +122,7 @@ mod tests {
corners: u8,
radius: u64
},
MultiShape {
Multi {
shapes: Vec<
enum Shape {
Inivisible,
Expand All @@ -132,7 +132,7 @@ mod tests {
corners: u8,
radius: u64
},
MultiShape {
Multi {
shapes: Vec<Shape>,
t: u64,
operation: enum Operation {
Expand Down
70 changes: 10 additions & 60 deletions description/src/transformer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{cell::RefCell, collections::HashMap};

use scale_info::{form::PortableForm, PortableRegistry, Type, TypeDef};
use scale_info::{form::PortableForm, PortableRegistry, Type};

/// The transformer provides an abstraction for traversing a type registry
/// given a type_id as a starting point, and **transforming** it into a tree-like structure (type parameter `R`).
Expand All @@ -27,6 +27,7 @@ pub struct Transformer<'a, R, S = ()> {
/// A cache hit is, when the representation of a type has already been computed.
///
/// You can return None to sidestep recursion protection and let the transformer continue.
#[allow(clippy::type_complexity)]
cache_hit_policy: fn(u32, &Type<PortableForm>, &R, &Self) -> Option<anyhow::Result<R>>,
registry: &'a PortableRegistry,
}
Expand All @@ -44,6 +45,7 @@ where
R: Clone + std::fmt::Debug,
{
/// Create a new transformer.
#[allow(clippy::type_complexity)]
pub fn new(
policy: fn(u32, &Type<PortableForm>, &Self) -> anyhow::Result<R>,
recurse_policy: fn(u32, &Type<PortableForm>, &Self) -> Option<anyhow::Result<R>>,
Expand Down Expand Up @@ -72,21 +74,15 @@ where
type_id
))?;

match self.cache.borrow().get(&type_id) {
Some(cache_value) => {
let result_or_continue = match cache_value {
Cached::Recursive => (self.recurse_policy)(type_id, ty, self),
Cached::Computed(repr) => (self.cache_hit_policy)(type_id, ty, repr, self),
};

if let Some(result) = result_or_continue {
return result;
}
if let Some(cache_value) = self.cache.borrow().get(&type_id) {
let result_or_continue = match cache_value {
Cached::Recursive => (self.recurse_policy)(type_id, ty, self),
Cached::Computed(repr) => (self.cache_hit_policy)(type_id, ty, repr, self),
};
if let Some(result) = result_or_continue {
return result;
}
Some(Cached::Computed(repr)) => {}
_ => {}
};

self.cache.borrow_mut().insert(type_id, Cached::Recursive);
let r = (self.policy)(type_id, ty, self)?;
self.cache
Expand All @@ -95,49 +91,3 @@ where
Ok(r)
}
}

/// Returns true for types where recursion should continue, instead of being stopped when recursion in being detected.
///
/// ## Background:
///
/// There is a problem in generating recursive type descriptions:
/// Suppose we have the following setup:
/// ```rust
/// struct A {
/// bees: Vec<B>
/// }
///
/// struct B {
/// id: u8,
/// others: Vec<B>
/// }
/// ```
/// This could be described as:
/// ```txt,no_run
/// struct A {
/// bees: Vec<struct B {
/// id: u8,
/// others: Vec<B>
/// }>
/// }
/// ```
/// But the recursive resolving would get stuck in the middle, reporting recursion.
/// This is because Vec<B> needs to be mapped to different strings, so the simple cache lookup is not viable.
/// The solution to this is, to just let some container types like Vec do recursion while others can't.
///
/// # Warning
///
/// The safety of the following logic relies on the assumption that ultimately everything resolves down to a primitive or a struct/enum that is in the cache.
/// It basically just returns true for generic wrapper types.
fn recursion_should_continue(def: &TypeDef<PortableForm>) -> bool {
match def {
scale_info::TypeDef::Sequence(_) => true,
scale_info::TypeDef::Array(_) => true,
scale_info::TypeDef::Tuple(_) => true,
scale_info::TypeDef::Compact(_) => true,
scale_info::TypeDef::Composite(_) => false,
scale_info::TypeDef::Primitive(_) => false,
scale_info::TypeDef::Variant(_) => false,
scale_info::TypeDef::BitSequence(_) => false,
}
}
8 changes: 4 additions & 4 deletions description/src/type_example/rust_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,10 @@ pub fn example_from_seed(

/// Note: because None is returned here, the transformer will just continue its work.
fn compute_another_example(
type_id: u32,
ty: &Type<PortableForm>,
cached_value: &TokenStream,
transformer: &CodeTransformer,
_type_id: u32,
_ty: &Type<PortableForm>,
_cached_value: &TokenStream,
_transformer: &CodeTransformer,
) -> Option<anyhow::Result<TokenStream>> {
None
}
Expand Down
8 changes: 4 additions & 4 deletions description/src/type_example/scale_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ pub fn example_from_seed(id: u32, types: &PortableRegistry, seed: u64) -> anyhow

/// Note: because None is returned here, the transformer will just continue its work.
fn compute_another_example(
type_id: u32,
ty: &Type<PortableForm>,
cached_value: &Value,
transformer: &ValueTransformer,
_type_id: u32,
_ty: &Type<PortableForm>,
_cached_value: &Value,
_transformer: &ValueTransformer,
) -> Option<anyhow::Result<Value>> {
None
}
Expand Down

0 comments on commit cfec37d

Please sign in to comment.