Add BitVec support #98
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Robbepop
approved these changes
Jun 21, 2021
src/ty/mod.rs
Outdated
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | ||
| #[cfg_attr(any(feature = "std", feature = "decode"), derive(scale::Decode))] | ||
| #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Debug)] | ||
| pub struct TypeDefBitVec<T: Form = MetaForm> { |
There was a problem hiding this comment.
I'd rename this to TypeDefBitSequence to match with Sequence type definition.
Robbepop
reviewed
Jun 21, 2021
src/ty/mod.rs
Outdated
Comment on lines
207
to
209
| #[cfg(feature = "bit-vec")] | ||
| /// A BitVec type. | ||
| BitVec(TypeDefBitVec<T>), |
There was a problem hiding this comment.
I think dtolnay mentioned that for compatibility with respect to different crate feature sets it is better to replace the inner type of BitVec with a placeholder type (dummy) of not(feature = "bit-vec") instead of entirely removing this variant.
There was a problem hiding this comment.
That is much better, will avoid issues with incompatible decoding if we add variants after this.
There was a problem hiding this comment.
I've removed the feature flag from this variant now so it will be binary compatible, and also so that decoding/deserializing is supported even without the feature enabled.
dvdplm
approved these changes
Jun 21, 2021
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Required for
polkadot, some types useBitVece.g. https://github.com/paritytech/polkadot/blob/master/primitives/src/v1/mod.rs#L468.Rationale is that
parity-scale-codechas support for bitvec: https://github.com/paritytech/parity-scale-codec/blob/master/src/bit_vec.rs, by enabling thebit-vecfeature, so this library should too.EDIT
I've modified this so that the
TypeDefBitSequencestruct and variant is still included even withoutbit-vecenabled, in order for compatible encoding/decoding between the crate with or without the feature enabled. Enabling the feature will simply add thebitvecdependency and the relevantTypeInfoimpls.