Relax BoundedVec trait restrictions
#8749
Conversation
A normal `Vec<T>` can do many things without any particular trait bounds on `T`. This commit relaxes the bounds on `BoundedVec<T, S>` to give it similar capabilities.
coriolinus
added
A0-please_review
| #[derive(Encode, Decode, crate::DefaultNoBound, crate::CloneNoBound, crate::DebugNoBound)] | ||
| pub struct BoundedVec<T: BoundedVecValue, S: Get<u32>>(Vec<T>, PhantomData<S>); | ||
| #[derive(Encode, Decode)] | ||
| pub struct BoundedVec<T, S>(Vec<T>, PhantomData<S>); |
There was a problem hiding this comment.
What's the benefit of relaxing S? I can't see why and how that should ever be anything other than Get<_>? is it just to avoid writing it in a few places? I'd argue that's not worth it given you now need to split the impl<T, S> BoundedVec<T, S> into two chunks.
There was a problem hiding this comment.
I mean, yes, it's so that we don't have to write it down in as many places. Agree that S is never going to be anything other than Get<_>, but there are quite a few impls which just don't care about S, so there's not much point in enforcing the restriction there.
I suspect, but cannot prove, that more-general generics are slightly faster to compile: the compiler doesn't need to do as much work proving that the bounds are satisfied.
There was a problem hiding this comment.
In the meantime I looked a bit more in the std-library, and it indeed seems like the correct paradigm is to name as few generics as possible, so you are indeed aligned with the idiomatic way of writing this kind of stuff.
What I don't get and disagree with, is that it seems like this rule of thumb is stretched the extent that it even doesn't make sense anymore 😬
But anyway, thanks for the explanation. I am unfamiliar with some idioms.
There was a problem hiding this comment.
The point is, never bound a generic to a trait in the declaration if that is not required by the declaration. There are some exceptions of this rule, but here it is completely fine to drop it.
…vec-trait-restrictions
|
bot merge |
|
Trying merge. |
ghost
merged commit 
jakoblell
added
D1-audited 👍
* requiring users to maintain an unchecked invariant is unsafe * relax trait restrictions on BoundedVec<T, S> A normal `Vec<T>` can do many things without any particular trait bounds on `T`. This commit relaxes the bounds on `BoundedVec<T, S>` to give it similar capabilities.
A normal
Vec<T>can do many things without any particular traitbounds on
T. This PR relaxes the bounds onBoundedVec<T, S>to give it similar capabilities.