-
Notifications
You must be signed in to change notification settings - Fork 443
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.
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
[storage] Allow one variant enum to derive SpreadLayout #942
Conversation
I have an enum that has only one variant, and I want to store the enum value in the storage. I might add new variants later in development, so the enum is not useless. The current implementation doesn't allow to derive the `SpreadLayout` trait for one variant enums, and I fixed it on this PR. # Sample code ```rust #![cfg_attr(not(feature = "std"), no_std)] use ink_lang as ink; #[derive( Copy, Clone, scale::Encode, scale::Decode, ink_storage::traits::SpreadLayout, ink_storage::traits::PackedLayout, )] #[cfg_attr( feature = "std", derive(scale_info::TypeInfo, ink_storage::traits::StorageLayout) )] pub enum MyEnum { A, } #[ink::contract] mod enum_test { use super::MyEnum; #[ink(storage)] pub struct EnumTest { value: MyEnum, } impl EnumTest { #[ink(constructor)] pub fn new() -> Self { Self { value: MyEnum::A } } #[ink(message)] pub fn get(&self) -> MyEnum { self.value } } } ``` Without this change, I get this error. ``` $ cargo +nightly contract build [1/5] Building cargo project Updating crates.io index Compiling enum_test v0.1.0 (/private/var/folders/zn/l2f569z56vnghtt524x1mv6w0000gn/T/cargo-contract_FM50JF) error: proc-macro derive panicked --> /snip/enum_test/lib.rs:10:5 | 10 | ink_storage::traits::SpreadLayout, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: message: can only operate on enums ```
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.
Thank you for the PR. All in all LGTM.
Co-authored-by: Robin Freyler <robbepop@web.de>
Codecov Report
@@ Coverage Diff @@
## master #942 +/- ##
==========================================
- Coverage 82.88% 82.86% -0.03%
==========================================
Files 177 177
Lines 8058 8059 +1
==========================================
- Hits 6679 6678 -1
- Misses 1379 1381 +2
Continue to review full report at Codecov.
|
Thank you for your review. CI passed. |
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.
LGTM
Thank you a lot for writing the test case!
This PR really is an improvement. :)
* [storage] Allow one variant enum to derive SpreadLayout I have an enum that has only one variant, and I want to store the enum value in the storage. I might add new variants later in development, so the enum is not useless. The current implementation doesn't allow to derive the `SpreadLayout` trait for one variant enums, and I fixed it on this PR. # Sample code ```rust #![cfg_attr(not(feature = "std"), no_std)] use ink_lang as ink; #[derive( Copy, Clone, scale::Encode, scale::Decode, ink_storage::traits::SpreadLayout, ink_storage::traits::PackedLayout, )] #[cfg_attr( feature = "std", derive(scale_info::TypeInfo, ink_storage::traits::StorageLayout) )] pub enum MyEnum { A, } #[ink::contract] mod enum_test { use super::MyEnum; #[ink(storage)] pub struct EnumTest { value: MyEnum, } impl EnumTest { #[ink(constructor)] pub fn new() -> Self { Self { value: MyEnum::A } } #[ink(message)] pub fn get(&self) -> MyEnum { self.value } } } ``` Without this change, I get this error. ``` $ cargo +nightly contract build [1/5] Building cargo project Updating crates.io index Compiling enum_test v0.1.0 (/private/var/folders/zn/l2f569z56vnghtt524x1mv6w0000gn/T/cargo-contract_FM50JF) error: proc-macro derive panicked --> /snip/enum_test/lib.rs:10:5 | 10 | ink_storage::traits::SpreadLayout, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: message: can only operate on enums ``` * Update crates/storage/derive/src/spread_layout.rs Co-authored-by: Robin Freyler <robbepop@web.de> * cargo fmt Co-authored-by: Robin Freyler <robbepop@web.de>
* Bump scale-info requirement to 0.9 * Update manual TypeInfo impls * Use decode feature of scale-info * scale-info 1.0 * Fix tests, 0 indexing on type registry * Update some missing examples * TMP: use ink rc5 for compat with cargo-contract * Update RELEASES.md * Bump versions back to 1.0 * Introduce versioning of ink metadata format * Update RELEASES.md Co-authored-by: Michael Müller <michi@parity.io> * Update crates/metadata/src/lib.rs Co-authored-by: Michael Müller <michi@parity.io> * Clippy: allow large enum variant * [storage] Allow one variant enum to derive SpreadLayout (#942) * [storage] Allow one variant enum to derive SpreadLayout I have an enum that has only one variant, and I want to store the enum value in the storage. I might add new variants later in development, so the enum is not useless. The current implementation doesn't allow to derive the `SpreadLayout` trait for one variant enums, and I fixed it on this PR. # Sample code ```rust #![cfg_attr(not(feature = "std"), no_std)] use ink_lang as ink; #[derive( Copy, Clone, scale::Encode, scale::Decode, ink_storage::traits::SpreadLayout, ink_storage::traits::PackedLayout, )] #[cfg_attr( feature = "std", derive(scale_info::TypeInfo, ink_storage::traits::StorageLayout) )] pub enum MyEnum { A, } #[ink::contract] mod enum_test { use super::MyEnum; #[ink(storage)] pub struct EnumTest { value: MyEnum, } impl EnumTest { #[ink(constructor)] pub fn new() -> Self { Self { value: MyEnum::A } } #[ink(message)] pub fn get(&self) -> MyEnum { self.value } } } ``` Without this change, I get this error. ``` $ cargo +nightly contract build [1/5] Building cargo project Updating crates.io index Compiling enum_test v0.1.0 (/private/var/folders/zn/l2f569z56vnghtt524x1mv6w0000gn/T/cargo-contract_FM50JF) error: proc-macro derive panicked --> /snip/enum_test/lib.rs:10:5 | 10 | ink_storage::traits::SpreadLayout, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: message: can only operate on enums ``` * Update crates/storage/derive/src/spread_layout.rs Co-authored-by: Robin Freyler <robbepop@web.de> * cargo fmt Co-authored-by: Robin Freyler <robbepop@web.de> * CI: remove gcda files from cache (#943) * Update pretty_assertions requirement from 0.7.1 to 1.0.0 (#944) Updates the requirements on [pretty_assertions](/~https://github.com/colin-kiegel/rust-pretty-assertions) to permit the latest version. - [Release notes](/~https://github.com/colin-kiegel/rust-pretty-assertions/releases) - [Changelog](/~https://github.com/colin-kiegel/rust-pretty-assertions/blob/main/CHANGELOG.md) - [Commits](rust-pretty-assertions/rust-pretty-assertions@v0.7.1...v1.0.0) --- updated-dependencies: - dependency-name: pretty_assertions dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add 'versioning' to spellcheck dict Co-authored-by: Michael Müller <michi@parity.io> Co-authored-by: tash-2s <81064017+tash-2s@users.noreply.github.com> Co-authored-by: Robin Freyler <robbepop@web.de> Co-authored-by: Denis Pisarev <denis.pisarev@parity.io> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
I have an enum that has only one variant, and I want to store the enum value in the storage. I might add new variants later in development, so the enum is not useless.
The current implementation doesn't allow to derive the
SpreadLayout
trait for one variant enums, and I fixed it on this PR.Sample code
Without this change, I get this error.