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

Do not use std imports #949

Conversation

pmikolajczyk41
Copy link
Contributor

Substitute std:: imports in the generated code with core:: or alloc:: to enable using macro within no-std context.

@pmikolajczyk41 pmikolajczyk41 requested a review from a team as a code owner May 9, 2023 08:29
@pmikolajczyk41 pmikolajczyk41 marked this pull request as draft May 9, 2023 12:17
@@ -218,6 +218,7 @@ impl RuntimeGenerator {
let types_mod = type_gen.generate_types_mod()?;

Ok(quote! {
extern crate alloc;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should move extern crate alloc to codegen/src/lib.rs

Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey,

In order to make this work in no-std, we must migrate from using std::io:Error (feature-gate it)
Thus, currently it does only work in std and I think migrating to no-std is quite a big task.

Can you write up an issue why you need this in no-std?
Even for WASM support subxt is using std but I guess it could useful in when doing offline or something?

@jsdw
Copy link
Collaborator

jsdw commented May 11, 2023

The reasoning for no_std comes from this PR:

use-ink/ink#1776

The idea as I understand it from chats with @pmikolajczyk41 is that it may be nice to be able to provide to contracts a custom environment containing a RuntimeCall type, so that contracts can use it to more easily encode calls to other pallets.

In this case, only the runtime types and none of the actual subxt interface stuff needs to be no_std compatible (ie you'd use runtime_types_only when calling the macro), because only the types are desired and not the rest of the interface code.

My general feeling is that if we can make the types that are generated no_std compatible without much fuss then I'd be up for it, but if it becomes more fiddly/annoying code wise to work out then I'd start leaning towards a separate crate being better to generate types in the specific way desired to use in contracts stuff (because ultimately I do view this subxt_macro crate as a part of subxt and don't want to head too far down supporting standalone use cases and the increased complexity that would come from that; my focus is mostly on making this crate do what we want to make it work well with the rest of Subxt).

Anyway, step one is seeing whether the contracts guys want a RuntimeCall type being part of this CustomEnvironment trait in the first place. If they do, then we can see whether it's not too hard to get subxt_macro to do what we want or whether it would be better to just suggest making a separate crate that is focused on that one use case.

@ascjones
Copy link
Contributor

Anyway, step one is seeing whether the contracts guys want a RuntimeCall type being part of this CustomEnvironment trait in the first place. If they do, then we can see whether it's not too hard to get subxt_macro to do what we want or whether it would be better to just suggest making a separate crate that is focused on that one use case.

Access to the types generated by the pallet calls is something we want indeed.

My general feeling is that if we can make the types that are generated no_std compatible without much fuss then I'd be up for it, but if it becomes more fiddly/annoying code wise to work out then I'd start leaning towards a separate crate being better to generate types in the specific way desired to use in contracts stuff (because ultimately I do view this subxt_macro crate as a part of subxt and don't want to head too far down supporting standalone use cases and the increased complexity that would come from that; my focus is mostly on making this crate do what we want to make it work well with the rest of Subxt).

Agree with this. If we are doing runtime_types_only I think it will be sufficient to just replace the usages of std that we already see in this PR?

I am just catching up with everything around this work, but from what I understand I don't see any big blockers to getting something working

@pmikolajczyk41 pmikolajczyk41 deleted the pmikolajczyk41/no-std-imports branch May 12, 2023 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants