-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Allow to name the generic for storages in #[pallet::storage]
#8751
Conversation
Why are we allowed to drop this here: Obviously it is preferred to not have |
Actually the exact specification should be discussed, this is my thoughts:
|
@thiolliere No commands could be detected from your comment. |
Would this be allowed for a regular rust type? |
When you for example have some trait and want to set the associated types in a where bound, you can also give them in any order. Requiring an order for stuff that is uniquely named is weird and just complicates the whole process again. |
the other idea which I don't prefer: type Foo<T> = StorageMap<_, Key<Twox128Concat, u32>, ValueNonOption<u32, OnEmpty3U32>, MaxValues<ConstU32<300>>>;
type Foo2<T> = StorageMap<_, Key<Twox128Concat, u32>, ValueNonOption<u32>, MaxValues<ConstU32<300>>>;
type Foo3<T> = StorageMap<_, Key<Twox128Concat, u32>, ValueOption<u32>, MaxValues<ConstU32<300>>>; But I think named generic is more understandable than rust wrapper which add one layer of complexity in the types. |
yeah i agree, what exists now is better |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nitpicks, otherwise looks good
error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied | ||
--> $DIR/storage_ensure_span_are_ok_on_wrong_gen.rs:20:12 | ||
| | ||
20 | #[pallet::storage] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not such a good diagnostic. Can we not use the correct span here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes we can point to the type alias instead of the storage attribute.
note that this not due to this PR, it is because we use the span of the attribute when expanding the storage metadata and getter. I think we can do in a follow-up i'll do an issue and look at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to improve a bit in this draft #8850
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! :)
frame/support/test/tests/pallet_ui/storage_value_unexpected_named_generic.stderr
Show resolved
Hide resolved
bot merge |
Trying merge. |
…ytech#8751) * implement named generic for storages * fix error message on unexpected name for generic
This PR make the macro handle to give named generics.
So you can declare like this:
NOTE: no need to write
Prefix = _,
NOTE: no need to declare them in order, and no need to declare all of them. For instance if user want to set
OnEmpty
generic, it can avoid declaringQueryKind
, the default query kind will be expanded by the macrocontext
This is motivated from feedback about readability.
This is also motivated in order to add yet another generic for storages:
MaxValues
which would tell the storage the expected number of max values #8729 #8735