Skip to content

Procedural macro for deriving Externals implementation#164

Closed
pepyakin wants to merge 25 commits intomasterfrom
derive
Closed

Procedural macro for deriving Externals implementation#164
pepyakin wants to merge 25 commits intomasterfrom
derive

Conversation

@pepyakin
Copy link
Collaborator

@pepyakin pepyakin commented Jan 25, 2019

This PR introduces wasmi-derive: a crate that allows users to generate implementations of Externals that wraps a given impl block.

Here is an example:

struct NonStaticExternals<'a> {
    state: &'a mut usize,
}

#[derive_externals]
impl<'a> NonStaticExternals<'a> {
    pub fn hello(&self, a: u32, b: u32) -> u32 {
        a + b
    }

    pub fn increment(&mut self) {
        *self.state += 1;
    }

    pub fn traps(&self) -> Result<(), NoInfoError> {
        Err(NoInfoError)
    }
}

the [derive_externals] macro will generate a Externals implementation for NonStaticExternals<'a> and it will also add inherent method called resolver for getting an instance of ModuleImportResolver.

Types that implement wasmi::derive_support::IntoWasmValue are supported in argument type positions and types that implement wasmi::derive_support::IntoWasmResult are supported in return type positions.

The macro is simple by itself, however it generates code that depends on type-inference and uses a couple of tricks.

@pepyakin pepyakin changed the title Derive Procedural macro for deriving Externals implementation Jan 25, 2019
@ddorgan
Copy link

ddorgan commented Jan 25, 2019

[clabot:check]

@parity-cla-bot
Copy link

It looks like @pepyakin signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

}
_ => panic!("unknown function index"),
#[wasmi_derive::derive_externals]
impl<'a> Runtime<'a> {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this example tested anywhere?

because otherwise there are no tests!

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, there will no longer be example for implementing host without procedural macro?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, except from tests.

I don't think it is worthwhile to keep implementing Externals manually here since this example doesn't do anything interesting and can be easily changed with the proc-macro.

@ddorgan
Copy link

ddorgan commented Jan 25, 2019

[clabot:check]

@parity-cla-bot
Copy link

It looks like @pepyakin signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@ddorgan
Copy link

ddorgan commented Jan 25, 2019

[clabot:check]

@parity-cla-bot
Copy link

It looks like @pepyakin signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

This way it shouldn't matter if anybody e.g. imports other `Option` or redefines `Result`.
pub ty: Box<syn::Type>,
}

/// Parse an incoming stream of tokens into externalities definition.
Copy link
Contributor

Choose a reason for hiding this comment

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

what is "externalities" in wasmi terms?

wouldn't even "externals" be a foreign term for WebAssembly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I'd agree that this term migrated from other projects here. What would you propose here?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe "host bindings", "host functions" or simply "host" (if it later can be modified that way that it works not only with functions) to keep with WebAssembly terminology

also, the attribute name (now "derive_externals") might also reflect this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, but I'm afraid we are really too far down the road to do that. There is already Externals trait. But yeah would be up to change the doc.

)
}

pub struct CompileError {
Copy link

Choose a reason for hiding this comment

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

You probably just want to use this :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks a lot, nice to know!

@pepyakin
Copy link
Collaborator Author

pepyakin commented Feb 7, 2019

Low priority for now, closing...

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.

5 participants