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.

Sign up for GitHub

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

Jump to bottom

Remove requirement on bitvec for std feature and constructing TypeDefBitSequence. #168

Merged
merged 7 commits into from Sep 13, 2022
Merged

Remove requirement on bitvec for std feature and constructing TypeDefBitSequence. #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ee718c0
Don't force bitvec to be included when "std" is enabled.
jsdw Sep 12, 2022
30f093f
update UI test output
jsdw Sep 12, 2022
31748e4
Ah, we test on nightly
jsdw Sep 12, 2022
6aedf4f
don't require bit-vec to construct a TypeDefBitSequence
jsdw Sep 12, 2022
033578c
no bit-vec feature on std actually
jsdw Sep 12, 2022
c96abd4
update test output again
jsdw Sep 12, 2022
3804c26
bump rust-version to 1.60.0 and add section in unreleased
jsdw Sep 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed
- Minimum Rust version is 1.60.0 owing to using weak dependency crate features.
- Loosen restriction on `TypeDefBitSequence::new()` so that `bitvec` isn't required, and try to avoid pulling in `bitvec` when the `std` feature is enabled [(#168)](https://github.com/paritytech/scale-info/pull/168)

## [2.1.2] - 2022-05-18

### Fixed
Expand Down
4 changes: 2 additions & 2 deletions Cargo.toml
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@ name = "scale-info"
version = "2.1.2"
authors = ["Parity Technologies <admin@parity.io>"]
edition = "2021"
rust-version = "1.56.1"
rust-version = "1.60.0"

license = "Apache-2.0"
readme = "README.md"
Expand All @@ -25,7 +25,7 @@ scale = { package = "parity-scale-codec", version = "3", default-features = fals
[features]
default = ["std"]
std = [
"bitvec/std",
"bitvec?/std",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bitvec crate will no longer be pulled in when "std" is enabled, but this I think makes no difference to the external interface since bitvec additions are behind the separate "bit-vec" feature anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that bitvec will still ultimately be pulled in via scale/std https://github.com/paritytech/parity-scale-codec/blob/b5f2b360e2939e8a235423f40591db09e879fdbb/Cargo.toml#L41

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh ok; I'll do a quick follow PR there then to prevent that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR for parity-scale-codec thing: paritytech/parity-scale-codec#375

"scale/std",
]
derive = [
Expand Down
16 changes: 10 additions & 6 deletions src/ty/mod.rs
View file
Open in desktop
Expand Up @@ -576,17 +576,21 @@ where
}
}

#[cfg(feature = "bit-vec")]
impl TypeDefBitSequence {
/// Creates a new [`TypeDefBitSequence`] for the supplied bit order and bit store types.
pub fn new<T, O>() -> Self
///
/// With the `bit-vec` feature enabled, the expected usage is to provide either
/// `bitvec::order::Lsb0` or `bitvec::order::Msb0` as the order type, and then something
/// like u8, u8, or u32 as the store type. Without the `bit-vec` feature enabled, it's
/// recommended that your types have identical `TypeInfo` to those.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…or else? :)
Like, is "recommended" the right word here? Isn't it more "has to be"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, in theory it can be whatever it wants; it's just that our current tooling won't know how to decode the thing if the types don't line up with what we expect (no reason somebody else can't make tooling that can support more exotic types though if they want to go that route)

pub fn new<Store, Order>() -> Self
where
T: bitvec::store::BitStore + TypeInfo + 'static,
O: bitvec::order::BitOrder + TypeInfo + 'static,
Store: TypeInfo + 'static,
Order: TypeInfo + 'static,
{
Self {
bit_store_type: MetaType::new::<T>(),
bit_order_type: MetaType::new::<O>(),
bit_store_type: MetaType::new::<Store>(),
bit_order_type: MetaType::new::<Order>(),
}
}
}
6 changes: 3 additions & 3 deletions test_suite/tests/ui/fail_missing_derive.stderr
View file
Open in desktop
@@ -1,8 +1,8 @@
error[E0277]: the trait bound `PawType<u16>: TypeInfo` is not satisfied
--> tests/ui/fail_missing_derive.rs:19:5
--> tests/ui/fail_missing_derive.rs:19:24
|
19 | assert_type_info::<Cat<bool, u8, u16>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `TypeInfo` is not implemented for `PawType<u16>`
| ^^^^^^^^^^^^^^^^^^ the trait `TypeInfo` is not implemented for `PawType<u16>`
|
= help: the following other types implement trait `TypeInfo`:
&T
Expand All @@ -14,7 +14,7 @@ error[E0277]: the trait bound `PawType<u16>: TypeInfo` is not satisfied
(A, B, C, D, E)
(A, B, C, D, E, F)
and 54 others
note: required because of the requirements on the impl of `TypeInfo` for `Cat<bool, u8, u16>`
note: required for `Cat<bool, u8, u16>` to implement `TypeInfo`
--> tests/ui/fail_missing_derive.rs:8:10
|
8 | #[derive(TypeInfo)]
Expand Down