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 5 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
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Expand Up @@ -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>(),
}
}
}
8 changes: 4 additions & 4 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 @@ -13,8 +13,8 @@ error[E0277]: the trait bound `PawType<u16>: TypeInfo` is not satisfied
(A, B, C, D)
(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>`
and 57 others
note: required for `Cat<bool, u8, u16>` to implement `TypeInfo`
--> tests/ui/fail_missing_derive.rs:8:10
|
8 | #[derive(TypeInfo)]
Expand Down
2 changes: 1 addition & 1 deletion test_suite/tests/ui/fail_unions.stderr
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@ error[E0277]: the trait bound `Commonwealth: TypeInfo` is not satisfied
(A, B, C, D)
(A, B, C, D, E)
(A, B, C, D, E, F)
and 53 others
and 56 others
note: required by a bound in `assert_type_info`
--> tests/ui/fail_unions.rs:11:24
|
Expand Down