Fix codegen for codec::Compact as type parameters
#651
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsdw
reviewed
Sep 16, 2022
| @@ -9,7 +9,6 @@ format_code_in_doc_comments = false | |||
| comment_width = 80 | |||
| normalize_comments = true # changed | |||
| normalize_doc_attributes = false | |||
| license_template_path = "FILE_TEMPLATE" # changed | |||
There was a problem hiding this comment.
I never really noticed but does removing this mean no checking that the license header is at the top of files now?
There was a problem hiding this comment.
No it has been removed now so it doesn't do anything other than give a warning rust-lang/rustfmt#5103
jsdw
reviewed
Sep 16, 2022
codegen/src/types/tests.rs
Outdated
Comment on lines
378
to
380
| struct S { | ||
| a: Option<<u128 as codec::HasCompact>::Type>, | ||
| } |
There was a problem hiding this comment.
Is it worth adding tests to check that some of the other TypePath variants lead to the output you expect?
lexnv
approved these changes
Sep 20, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes use-ink/cargo-contract#734.
When a
codec::Compacttype appears as a type parameter e.g.Option<Compact<u128>>as it does in the contracts dispatchables e.g. https://github.com/paritytech/substrate/blob/4b7911607b48a5ca7ebf1d655f12d91cdd241866/frame/contracts/src/lib.rs#L498, then this will incorrectly result in aOption<u128>type being generated bysubxt-codegen.This is because the current codegen assumes that the compact value will be the type of the field directly and can generate the following:
struct S { #[codec(compact)] field: u128 }The
compactattribute will cause the conversion to be handled during encoding and decoding of the struct. This means that the "inner" type (in this caseu128) can be used directly.However when the compact type is a type parameter then it is required to use the
codec::Compact<T>wrapper type directly instead of the field level attribute.Refactoring
This PR also incorporates some refactoring of the type path codegen, specifically expanding the
TypePathTypeto a strongly typed enum which maps from thescale_info::TypeDefbut with all the information necessary to generate the hierarchical type paths.