Stream: git-wasmtime

Topic: wasmtime / issue #5473 cranelift: Support derived instruc...

Wasmtime GitHub notifications bot (Dec 19 2022 at 23:17):

jameysharp commented on issue #5473:

So this version requires anyone who wants to call the jump method to explicitly use the new InstBuilderDerived trait, right? We talked about trying to name the new trait InstBuilder and rename the auto-generated trait to something else. I take it that didn't work out?

How about this: make the generated inst_builder.rs contain only the method definitions, not the surrounding pub trait InstBuilder etc. Move the pub trait declaration to the hand-written code, so it can have hand-written methods in it, and stick include!(concat!(env!("OUT_DIR"), "/inst_builder.rs")); into the middle of that trait so all the generated methods are also in the same trait. Does that make this diff smaller?

Wasmtime GitHub notifications bot (Dec 20 2022 at 00:10):

elliottt commented on issue #5473:

So this version requires anyone who wants to call the jump method to explicitly use the new InstBuilderDerived trait, right? We talked about trying to name the new trait InstBuilder and rename the auto-generated trait to something else. I take it that didn't work out?

I found that the previous InstBuilder methods weren't available without importing it as well, which led me to give the derived functions a new name.

How about this: make the generated inst_builder.rs contain only the method definitions, not the surrounding pub trait InstBuilder etc. Move the pub trait declaration to the hand-written code, so it can have hand-written methods in it, and stick include!(concat!(env!("OUT_DIR"), "/inst_builder.rs")); into the middle of that trait so all the generated methods are also in the same trait. Does that make this diff smaller?

I think that would make the diff smaller! The downside would be that we'd still need to manually port the documentation over for any hand-written wrappers, as the docs wouldn't be automatically generated for them. I think I'm going to try a different approach, and generate the version of the builder that takes the Block and &[Value] arguments separately back on #5464. That will avoid any issues of documentation sync, and should make it easy enough to generate multiple versions of the same function if necessary.

Last updated: Oct 23 2024 at 20:03 UTC