-
Notifications
You must be signed in to change notification settings - Fork 248
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
Allow for remapping type parameters in type substitutions #735
Conversation
|SubstituteType { ty, with }| { | ||
( | ||
ty, | ||
with.try_into() |
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 can be quite cryptic, but I didn't want to import the AbsoluteTypePath
newtype wrapper that I introduced for the validation purpose (rather than it being a more well-rounded/useful type on its own), to name it directly here.
1a049b0
to
29f1ab5
Compare
use
statements for type substitutions
Squashed and force-pushed now after #760 landed. Implements variant specified in #666 (comment) (so uses new syntax as well). This now includes the ability to strip generics altogether for a source type or taking any combination of parameters from the source type. This lacks the ability to specify concrete types becase we'd need to plug into type resolution as part of the substitution but since this PR refactored type substitution and made it more self-contained and introduced the new mapping feature, I figured we could do that in a follow-up PR. |
@@ -115,13 +131,14 @@ pub fn generate_runtime_api_from_bytes( | |||
item_mod: syn::ItemMod, | |||
bytes: &[u8], | |||
derives: DerivesRegistry, | |||
type_substitutes: TypeSubstitutes, |
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.
Super small nit: missing
/// * `type_substitutes` - Provide custom type substitutes.
on the doc.
Tho, I'm wondering if we should extend this documentation style to our repo, or just remove those # Arguments
in the future. I guess the easiest would be later since we already have some comprehensive docs.
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.
Good spot!
I added the extra param for now, though I'd note that these comments aren't exposed publically anywhere and so in general I'd be inclined to keep them simple as they are only for us devs who work on it anyway :)
Closes #666
For now, this moves type substitution annotations from the use statements outside, to the
subxt
macro + separates some substitution logic. Opening as a draft for now to get feedback; next thing to work on is to implement the generic wrangling/reordering mentioned in #666.