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

Remove bitvec from subxt-codegen #693

Closed
jsdw opened this issue Oct 19, 2022 · 2 comments · Fixed by #718
Closed

Remove bitvec from subxt-codegen #693

jsdw opened this issue Oct 19, 2022 · 2 comments · Fixed by #718
Assignees
Labels
ready to implement No significant design questions remain; this can be implemented now

Comments

@jsdw
Copy link
Collaborator

jsdw commented Oct 19, 2022

Currently, we still use bitvec in the subxt codegen stuff (we generate specific store/order things when we encounter bitvec types). As BitVec can't be compiled to wasm with every store type we support, this means we may occasionally generate code that doesn't compile to WASM. And anyway, we don't use bitvec elsewhere now so it'd be good to get rid of it from everywhere.

So let's figure out how to do this! Possibly something like:

pub enum U8 {}
pub enum U16 {}
// ...

pub enum Lsb0 {}
pub enum Msb0 {}

#[derive(Debug, Clone)]
pub struct DecodedBits<Store, Order>(scale_bits::Bits)

impl Decode for DecodedBits<U8, Lsb0> { // ... use scale_bits to do the heavy lifting }
impl Decode for DecodedBits<U16, Lsb0> { // ... use scale_bits to do the heavy lifting }
// ...

// and then in the codegen, use those types in substitute, instead of swapping out the 
// bitvec store/order types to make valid bitvecs

This is a bit verbose of an approach; perhaps we can think of something simpler?

@jsdw jsdw added the ready to implement No significant design questions remain; this can be implemented now label Oct 19, 2022
@Xanewok Xanewok self-assigned this Oct 26, 2022
@Xanewok
Copy link
Contributor

Xanewok commented Oct 26, 2022

I understand that BitVec<U64, ...> is the problematic type since the support in bitvec relies on cfg(target_pointer_width = "64") but we also want to always handle BitVec-style types in general. What about usize as a store type?

Do we want to forward more impls to the inner type or should we just expose the inner field for the consumer directly?

@jsdw
Copy link
Collaborator Author

jsdw commented Oct 26, 2022

Fortunately, the only store types I think we need to care about are U8, U16, U32 and U64 as those are the types that parity_scale_codec::Decode and the bitvec::Store trait are implemented on iirc (people can create custom store and order types but shrug; I just want the default ones to be OK I guess!).

I'd probably just have the codegen expose publically the inner scale_bits::Bits type, and otherwise just support the basic traits on the wrapper type I guess to make it work OK in the codegen stuff :)

So maybe the type is more like pub struct DecodedBits<Store,Order>(pub scale_bits::Bits), and maybe that lives under the utils module in subxt and then subxt_codegen would end up generating code like:

::subxt::utils::bits::DecodedBits<::subxt::utils::bits::U8, ::subxt::utils::bits::Lsb0>

Or something?

I hope I answered your Q's!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to implement No significant design questions remain; this can be implemented now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants