Skip to content
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 substitute types to take on 0, 1 or all of the generic params from the types they stand in for #666

Closed
jsdw opened this issue Sep 23, 2022 · 5 comments · Fixed by #735
Assignees
Labels
ready to implement No significant design questions remain; this can be implemented now

Comments

@jsdw
Copy link
Collaborator

jsdw commented Sep 23, 2022

See #627 for initial details.

Basically, right now we can do something like:

#[subxt::subxt(runtime_metadata_path = "./metadata.scale")]
pub mod node {
    #[subxt(substitute_type = "interbtc_primitives::VaultId")]
    use crate::DefaultVaultId;
}

The effect of this is that instead of generating/using an interbtc_primitives::VaultId type, we'd use the crate::DefaultValueId instead.

If the interbtc_primitives::VaultId type has generic params, so it would look like eg struct VaultId<A, B, C>, then all of those generic params are assumed to exist on the substitute type; we'd expect a struct DefaultValueId<A, B, C>.

It would be useful to have more flexibility over how the generic parameters are mapped to the substitute type though (see #627 for one such issue statement).

Suggested syntax ideas:

1. pattern matching on generics.

#[subxt::subxt(runtime_metadata_path = "./metadata.scale")]
pub mod node {
    // The default case; any generic parameters are applied as-is to the substitute type:
    #[subxt(substitute_type = "interbtc_primitives::VaultId")]
    use crate::DefaultVaultId;

    // Assert that `interbtc_primitives::VaultId` has 3 generic params. We want to ignore them all:
    #[subxt(substitute_type = "interbtc_primitives::VaultId<_, _, _>")]
    use crate::DefaultVaultId;

    // Assert that `interbtc_primitives::VaultId` has 3 generic params. Perform the mapping
    // `interbtc_primitives::VaultId<_2, X, _1>` => `DefaultVaultId<_1, _2>`
    // Ie, pattern match on VauldId generics and use numbers to map them to the output position.
    #[subxt(substitute_type = "interbtc_primitives::VaultId<_2, _, _1>")]
    use crate::DefaultVaultId;
}

Advantages:

  • Close to current syntax.
  • Rust-like generic syntax might feel familiar.
  • Built-in assertion that the input type has some number of generic params; we can error if this expectation changes.

Disadvantages:

  • Personally I find it hard to parse/read the third case.
  • Not obvious that numbers mean something.

2. Add a "params" attr.

#[subxt::subxt(runtime_metadata_path = "./metadata.scale")]
pub mod node {
    // The default case; any generic parameters are applied as-is to the substitute type:
    #[subxt(substitute_type = "interbtc_primitives::VaultId")]
    use crate::DefaultVaultId;

    // We want to ignore all of the generic params and apply none of them to DefaultVaultId
    #[subxt(substitute_type = "interbtc_primitives::VaultId", substitute_params = "")]
    use crate::DefaultVaultId;

    // Use the "params" attr to map by position in VaultId to substitute param order.
    #[subxt(substitute_type = "interbtc_primitives::VaultId", substitute_params = "2, 1")]
    use crate::DefaultVaultId;
}

Advantages:

  • Perhaps a little easier to read than the above?

3. use a new attribute to perform substitution

#[subxt::subxt(runtime_metadata_path = "./metadata.scale")]
#[subxt(substitute(input = "interbtc_primitives::VaultId", output = "crate::DefaultVaultId"))]
#[subxt(substitute(input = "interbtc_primitives::VaultId<A,B,C>", output = "crate::DefaultVaultId"))]
#[subxt(substitute(input = "interbtc_primitives::VaultId<A,B,C>", output = "crate::DefaultVaultId<B, A>"))]
pub mod node { }

Advantages:

  • IMO the clearest sort of approach; we can be clear and obivous with how we are mapping since we control input and output paths fully.

Disadvantages:

  • Strays from the current approach, which there may have been a good reason for using in the first place.

@ascjones @gregdhill I'd love to get your feedback; do you like the idea, and if so, do you have a preference on which syntax to go for, or reasons against any?

@ascjones
Copy link
Contributor

ascjones commented Sep 23, 2022

Strays from the current approach, which there may have been a good reason for using in the first place.

No special reason other than I thought it intuitive to hijack the use syntax to import a type.

Option 3. though seems to be clearest to understand the mapping of the parameters. I suppose a 4th option would be to combine 2 and 3 like so:

#[subxt::subxt(runtime_metadata_path = "./metadata.scale")]
pub mod node {
    // The default case; any generic parameters are applied as-is to the substitute type:
    #[subxt(substitute_type = "interbtc_primitives::VaultId")]
    use crate::DefaultVaultId;

    // We want to ignore all of the generic params and apply none of them to DefaultVaultId
    #[subxt(substitute_type = "interbtc_primitives::VaultId", input_params = "A, B, C", output_params = "")]
    use crate::DefaultVaultId;

    // Use the "params" attr to map by position in VaultId to substitute param order.
    #[subxt(substitute_type = "interbtc_primitives::VaultId", input_params = "A, B, C", output_params = "B, A")]
    use crate::DefaultVaultId;
}

@jsdw
Copy link
Collaborator Author

jsdw commented Sep 23, 2022

I'm probably in favour of 3 still just because it feels clearest (input and output might have slightly better name choices though).

One question in my head with it; I wonder whether:

#[subxt(substitute(input = "interbtc_primitives::VaultId", output = "crate::DefaultVaultId"))]

Should fail if VaultId has generics, ie we have to match on name and generic params. I'm leaning now towards "yes, and we can provide a decent error message to correct the user if generic param count is wrong anyway".

@niklasad1
Copy link
Member

My cents on suggestion 3 as well.

@jsdw
Copy link
Collaborator Author

jsdw commented Sep 27, 2022

Ok, let's do 3, so the API will look something like

#[subxt::subxt(runtime_metadata_path = "./metadata.scale")]
#[subxt(substitute(input = "interbtc_primitives::VaultId", output = "crate::DefaultVaultId"))]
#[subxt(substitute(input = "interbtc_primitives::VaultId<A,B,C>", output = "crate::DefaultVaultId"))]
#[subxt(substitute(input = "interbtc_primitives::VaultId<A,B,C>", output = "crate::DefaultVaultId<B, A>"))]
pub mod node { }

and if the wrong number of generic arguments are provided in any case, we can throw up an error and tell the user that.

@jsdw jsdw added the ready to implement No significant design questions remain; this can be implemented now label Sep 27, 2022
@jsdw
Copy link
Collaborator Author

jsdw commented Sep 30, 2022

As a side node, a nice to have may also be supporting generic params that are in scope but don't exist on the source type, so:

#[subxt::subxt(runtime_metadata_path = "./metadata.scale")]
#[subxt(substitute(input = "interbtc_primitives::VaultId<A,B>", output = "crate::DefaultVaultId<crate::Foo, A>"))]
pub mod node { }

Would work, using some concrete type crate::Foo since no input generic param matches this (well, and in this example a path and not ident is used), but using A from the input generics since it matches one of those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to implement No significant design questions remain; this can be implemented now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants