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

Idea: More Conversion #63

Closed
blckngm opened this issue Jan 12, 2023 · 7 comments · Fixed by #80
Closed

Idea: More Conversion #63

blckngm opened this issue Jan 12, 2023 · 7 comments · Fixed by #80

Comments

@blckngm
Copy link

blckngm commented Jan 12, 2023

Many Pack/Unpack/PackVec functionalities can be integrated into molecule codegen using standard conversion traits:

  • FooOpt From Foo
  • FooVec FromIterator
  • Fixed length byte array Into/From [u8; len] (with the caveat that converting to [u8; len] may panic)
  • Fixed length byte array TryFrom &[u8]

WDYT?

@yangby-cryptape
Copy link
Collaborator

yangby-cryptape commented Jan 12, 2023

LGTM!

TODOs

  • impl From<Foo> for FooOpt {}
  • impl FromIterator<Foo> for FooVec {}
  • impl From<&[Foo]> for FooVec {}
  • impl From<[Foo; N]> for FooArray {}
  • impl TryFrom<&[Foo]> for FooArray {}
  • Since Rust 1.51 stabilized Const Generics MVP, I think the code for fixed length array could be rewritten.

p.s. I'm sorry to said that these features won't be implemented soon, but they are on the TODO list, we will do them asap.

@eval-exec
Copy link
Collaborator

eval-exec commented Feb 28, 2023

Consider there is a foo.mol molecule schema file which content is

array Foo [byte;2];

option FooOpt       (Foo);
vector FooVec       <Foo>;
array  FooArray     [Foo; 10];
array  FooArray_MVP [Foo; 10];

Since Rust MVP has already be stable. We can make FooArray_MVP's rust code be this:

pub struct FooArray_MVP<const N: usize>([Foo; N]);

then we can implement this for FooArray_MVP :

impl<const N: usize> From<[Foo; N]> for FooArray_MVP<N> {
    fn from(value: [Foo; N]) -> Self {
        FooArray_MVP(value)
    }
}

molecule's codegen/generator/rust can implement those traits like this:

impl From<Foo> for FooOpt {
    fn from(v: Foo) -> Self {
        FooOpt::new_builder().set(Some(v)).build()
    }
}

impl FromIterator<Foo> for FooVec {
    fn from_iter<T: IntoIterator<Item = Foo>>(iter: T) -> Self {
        let mut builder = FooVec::new_builder();
        for v in iter {
            builder = builder.push(v);
        }
        builder.build()
    }
}
impl From<&[Foo]> for FooVec {
    fn from(v: &[Foo]) -> Self {
        let mut builder = FooVec::new_builder();
        for v in v {
            builder = builder.push(v.clone());
        }
        builder.build()
    }
}

pub struct FooArray_MVP<const N: usize>([Foo; N]);

impl<const N: usize> From<[Foo; N]> for FooArray_MVP<N> {
    fn from(value: [Foo; N]) -> Self {
        FooArray_MVP(value)
    }
}

impl TryFrom<&[Foo]> for FooArray {
    type Error = molecule::error::VerificationError;
    fn try_from(value: &[Foo]) -> Result<Self, Self::Error> {
        let mut builder = FooArray::new_builder();
        let mut value: &[Foo; FooArray::ITEM_COUNT] = value.try_into().map_err(|_| {
            molecule::error::VerificationError::TotalSizeNotMatch(
                "total size not match".to_string(),
                FooArray::ITEM_COUNT,
                value.len(),
            )
        })?;
        builder.set(value.clone());
        Ok(builder.build())
    }
}

@blckngm
Copy link
Author

blckngm commented Dec 28, 2023

It's unclear to me how Const Generics can make fixed length array better. Maybe you should elaborate in a separate issue.

@blckngm
Copy link
Author

blckngm commented Dec 28, 2023

Most are implemented in #80, except for:

  • TryFrom<&[Foo]> for FooArray: it's often byte array so just use TryFrom<&[u8]> for [u8; n] first.
  • From<&[Foo]>: use foo_slice.iter().cloned().collect().

@blckngm
Copy link
Author

blckngm commented Dec 28, 2023

On second thought, type inference won't work for foo_slice.try_into().unwrap().into(), so it would be still nice to have TryFrom<&[Foo]> directly.

@blckngm
Copy link
Author

blckngm commented Dec 28, 2023

There's a complication: Byte is not u8. We often want to work with &[u8] or [u8; n], not &[Byte] or [Byte; n].

@blckngm
Copy link
Author

blckngm commented Dec 28, 2023

Also implemented conversion for [u8; n] / &[u8] / u8 iterator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants