-
Notifications
You must be signed in to change notification settings - Fork 258
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
Add 'Static' type and improve type substitution codegen to accept it #886
Merged
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
lexnv
reviewed
Mar 29, 2023
lexnv
reviewed
Mar 29, 2023
lexnv
approved these changes
Mar 29, 2023
niklasad1
reviewed
Mar 31, 2023
|
||
impl CodegenError { | ||
/// Fetch the location for this error. | ||
// Todo: Probably worth storing location outside of the variant, |
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.
can you file an issue this?
niklasad1
reviewed
Mar 31, 2023
niklasad1
reviewed
Mar 31, 2023
niklasad1
reviewed
Mar 31, 2023
niklasad1
reviewed
Mar 31, 2023
niklasad1
reviewed
Mar 31, 2023
niklasad1
reviewed
Mar 31, 2023
testing/ui-tests/src/incorrect/substitute_path_not_absolute.stderr
Outdated
Show resolved
Hide resolved
niklasad1
reviewed
Mar 31, 2023
niklasad1
approved these changes
Mar 31, 2023
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.
this is great
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
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.
This PR:
subxt::utils::Static<T>
type If the typeT
impls Encode/Decode (ie static encoding and decoding), then theStatic
type itself implementsEncodeAsType
andDecodeAsType
(which it'll do by statically encoding and decoding).subxt::utils::Static
to make it possible to substitute in types which only implEncode
andDecode
(albeit wrapped in this Static wrapper).codegen
command #851).errors.rs
module to be more consistent, and extend them with TypeSubstitutionErrors.The overall reason for this is to help provide a path forwards for users who are currently using type substitution to pull in types which don't impl
EncodeAsType
andDecodeAsType
(eg types from sp_runtime/sp_core etc), now that those traits are required to be implemented on generated types.So a user can now do things like:
For instance, to use
sp_runtime
'sMultiAddress
instead of our own Subxt implementation.Note that, in general, it's better to rely on types which do impl
EncodeAsType
andDecodeAsType
where at all possible (Subxt provides autils::MultiAddress
andutils::AccountId32
for instance which do impl these traits).