Skip to content

wit-bindgen-rust: enable standalone code generation.#193

Merged
alexcrichton merged 1 commit into
bytecodealliance:mainfrom
peterhuene:update-rust-codegen
Jun 14, 2022
Merged

wit-bindgen-rust: enable standalone code generation.#193
alexcrichton merged 1 commit into
bytecodealliance:mainfrom
peterhuene:update-rust-codegen

Conversation

@peterhuene
Copy link
Copy Markdown
Member

@peterhuene peterhuene commented Apr 12, 2022

This PR enables a "standalone" mode to the Rust bindings generator.

The general idea is that this mode may be used to generate bindings that can be
built as separate crates and depended upon like normal Rust crates.

For both import and export bindings, this means not nesting the bindings inside
of a module.

For export bindings, this means generating a macro that consuming crates can
use to export an interface for a given type. Additionally, this allows users to
export multiple interfaces from the same type (e.g. a single Component struct).

An optional module field was added to Interface to also allow users some
control over the exported and imported symbol names. When specified, it is used
as the module name for import bindings. For export bindings, it is used as part
of a formatted export name of <module>#<function>; this format is what is
currently expected of wit-component for exported (non-default) interfaces.

@peterhuene
Copy link
Copy Markdown
Member Author

peterhuene commented Apr 12, 2022

Putting this up as a proposed solution to a few things:

  • Add the ability to generate bindings into static files that get compiled into referenced crates rather than generated compile-time with a proc-macro. This has the benefit of having source definitions for the bindings that users can view or (potentially) debug through and also better mimics how normal Rust dependencies are used via crates (with the end goal being editing a Cargo.toml is all users need to do to reference third party interfaces and components, much like "normal" dependencies).

  • Add the ability to easily export multiple interfaces from the same component. The solution here matches what is expected of wit-component in terms of mangled exported names from the core module (a simple <module>#<name> format).

  • Add the ability to better control the module name for imports or the part of manged name relating to the module for exports. This will enable users to import or export different versions of the same interface without name collisions.

Regarding the above, I'm completely open to alternative solutions; this was needed as currently proposed simply to make progress on the component tooling.

If we have the following interface:

say-something: function() -> string

Then the "standalone" generated code for this as an import would be:

pub fn say_something() -> String {
    unsafe {
        let ptr0 = IMPORT_RET_AREA.as_mut_ptr() as i32;
        #[link(wasm_import_module = "import-0.1.0")]
        extern "C" {
            #[cfg_attr(target_arch = "wasm32", link_name = "say-something")]
            #[cfg_attr(not(target_arch = "wasm32"), link_name = "import-0.1.0_say-something")]
            fn wit_import(_: i32);
        }
        wit_import(ptr0);
        let len1 = *((ptr0 + 8) as *const i32) as usize;
        String::from_utf8(Vec::from_raw_parts(
            *((ptr0 + 0) as *const i32) as *mut _,
            len1,
            len1,
        ))
        .unwrap()
    }
}
static mut IMPORT_RET_AREA: [i64; 2] = [0; 2];

And for the export:

/// Declares the export of the interface for the given type.
#[macro_export]
macro_rules! export(($t:ident) => {
  #[export_name = "export-0.1.0#say-something"]
  unsafe extern "C" fn __wit_bindgen_export_say_something() -> i32{
    #[allow(unused_imports)]
    use wit_bindgen_rust;
    use export::*;
    let result0 = <$t as Export>::say_something();
    let vec1 = (result0.into_bytes()).into_boxed_slice();
    let ptr1 = vec1.as_ptr() as i32;
    let len1 = vec1.len() as i32;
    core::mem::forget(vec1);
    let ptr2 = EXPORT_RET_AREA.as_mut_ptr() as i32;
    *((ptr2 + 8) as *mut i32) = len1;
    *((ptr2 + 0) as *mut i32) = ptr1;
    ptr2
  }
  static mut EXPORT_RET_AREA: [i64; 2] = [0; 2];
});
pub trait Export {
    fn say_something() -> String;
}

And used like this from a user's project, assuming import and export where somehow magically referenced to the generated crates on build:

use export::Export;

struct Component;

impl Export for Component {
    fn say_something() -> String {
        import::say_something()
    }
}

export::export!(Component); // one export macro invocation per exported interface

@peterhuene peterhuene requested a review from alexcrichton April 12, 2022 02:24
@peterhuene
Copy link
Copy Markdown
Member Author

I should also add that the changes here are entirely opt-in and used by the component tooling, not the proc-macros. The existing proc-macro-based generation output should remain the same.

@alexcrichton
Copy link
Copy Markdown
Member

This all seems pretty reasonable to me! I'd prefer though to not maintain multiple modes if we can. Could the new mode be just how the proc-macro works?

@peterhuene
Copy link
Copy Markdown
Member Author

I think I can remove the standalone option if we unconditionally use a wrapping mod on the generated code and the code generation for the component tooling can simply append a pub use foo::* to reexport everything at the crate-level.

The rest of the changes can remain and we'll need to update the examples/tests to use the generated export macros:

wit_bindgen_rust::export!("export.wit");
wit_bindgen_rust::import!("import.wit");

struct Component;

impl export::Export for Component {
    fn say_something() -> String {
        import::say_something()
    }
}

export::export!(Component); // one export macro invocation per exported interface

How does that sound?

@alexcrichton
Copy link
Copy Markdown
Member

Sounds reasonable to me!

@peterhuene
Copy link
Copy Markdown
Member Author

Argh...I've been trying to get this to play nice with resources and failing (not something necessary for the component model right this minute, so this PR punted on that for now with the "standalone" support); lots of assumptions on super in the codegen currently.

My first attempt was "oh I'll just add a type to the interface trait so that the type implementing the trait can specify resource implementation types" and that made some of the resource test cases happy.

But then I got to the handles test case where handles to resource types are inside of the type definitions and not just the exported function signatures; those need to exist outside of the export! macro and so there's no way to inform the type definitions of the implementer of the interface trait.

I'm still thinking about how best to accomplish this.

Comment thread crates/gen-rust-wasm/src/lib.rs Outdated
@Michael-F-Bryan
Copy link
Copy Markdown
Contributor

I like this export::export!(Component) approach!

I had been thinking of something similar for work so we can publish a helper crate that wraps WIT file imports and exports, plus some convenience functions, so downstream users just need to add a single crate as a dependency.

I think I can remove the standalone option if we unconditionally use a wrapping mod on the generated code and the code generation for the component tooling can simply append a pub use foo::* to reexport everything at the crate-level.

FWIW, you can do pub use foo::* to reexport the generated code from the current implementation. Is that what you were trying to achieve?

@peterhuene
Copy link
Copy Markdown
Member Author

peterhuene commented Apr 13, 2022

FWIW, you can do pub use foo::* to reexport the generated code from the current implementation. Is that what you were trying to achieve?

Effectively. I meant that I would put the generated mod back in whereas this PR currently removes it for the "standalone" mode because it desires the generated code to be put at the top-level of the generated crate.

By having the mod always present and just have the standalone tooling add an additional pub use foo::* to reexport everything at the top-level crate we'd be able to remove the "standalone" mode from the PR and still keep the desired semantics from the wit_bindgen_rust proc macros.

The problems I'm running into right now relate to what to do about resources as they also reference super in their code generation (primarily to use the actual resource type users are implementing).

If we didn't support resources (we don't yet in the component model, but will soon enough), then there would be no issues with removing the "standalone" mode from this PR using the technique described above.

@alexcrichton
Copy link
Copy Markdown
Member

I think it's an open option to removing resource support for now and re-adding it back in when the component model gets it. We'll probably have to solve the same questions you're running into now but we can hope that the intervening time gave us inspiration.

In general this is starting to age relative to the recent changes in the canonical ABI as well so it's due for a number of updates.

@theduke
Copy link
Copy Markdown
Contributor

theduke commented Apr 14, 2022

Great timing, I just also asked for this in #191 .

My idea there was to add an external = [...] setting to the proc macro that would disable code generation for the respective wit package, and instead expect a correctly named module to be in scope.

@peterhuene peterhuene force-pushed the update-rust-codegen branch 3 times, most recently from 534c910 to 4c5430f Compare April 23, 2022 00:09
@peterhuene peterhuene force-pushed the update-rust-codegen branch from 4c5430f to f737b3a Compare May 9, 2022 20:05
@theduke
Copy link
Copy Markdown
Contributor

theduke commented Jun 7, 2022

What's the status on this? Any blockers?

I'm happy to help out if needed.

This commit enables a "standalone" mode to the Rust bindings generator.

The general idea is that this mode may be used to generate bindings that can be
built as separate crates and depended upon like normal Rust crates.

For both import and export bindings, this means not nesting the bindings inside
of a module.

For export bindings, this means generating a macro that consuming crates can
use to export an interface for a given type. Additionally, this allows users to
export multiple interfaces from the same type.

An optional `module` field was added to `Interface` to also allow users some
control over the exported and imported symbol names. When specified, it is used
as the module name for import bindings. For export bindings, it is used as part
of a formatted export name of `<module>#<function>`; this format is what is
currently expected of `wit-component` for exported (non-default) interfaces.

This also removes support for the `crate_alias` option that was used in some
prototypes to internally alias the `wit_bindgen_rust` crate references; the
option is no longer necessary.
@peterhuene peterhuene force-pushed the update-rust-codegen branch from f737b3a to 25b7ee6 Compare June 14, 2022 17:58
@peterhuene
Copy link
Copy Markdown
Member Author

@theduke sorry, I missed your ping last week. I've been focused entirely on component model proposal work these past few weeks and haven't given this PR any attention. However, this PR doesn't quite implement what is needed to close out #191 since it just makes it easier for the code to live in separate crates and doesn't do the "use this from an external crate" part; baby steps, I think.

@alexcrichton it's been a little time since we last talked about this PR. It is currently what cargo-component depends on to do bindings code generation. Personally I'd prefer living with this additional "mode" in the generator for the short term since I have seen people exploring the use of resources and I'm uncomfortable removing them just because we can't get them to currently fit in this new export! macro implementation due to their reliance on super.

Plus, it's not that much in terms of change to back out at a later date once we settle on what code generation for resources should look like if we want to users might prefer to generate them into a external crate rather than inline from a proc macro.

What are your thoughts on this? If you're super opposed to it, I can keep cargo-component on a fork until we solve this all for real.

@alexcrichton
Copy link
Copy Markdown
Member

Ok that sounds reasonble to me yeah. Would you be ok writing up an issue on removing this option and using it by default so we can track that?

@peterhuene
Copy link
Copy Markdown
Member Author

@alexcrichton opened #243 to track that.

@alexcrichton alexcrichton merged commit 067661d into bytecodealliance:main Jun 14, 2022
@peterhuene peterhuene deleted the update-rust-codegen branch June 14, 2022 22:55
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